Re: CVS commit: src/sbin/amrctl

2020-09-06 Thread Jaromír Doleček
What's wrong with printf("%s", NULL)? It produces '(null)', and at
least it's visible something is missing there. I think gcc 9.3 is
overly eager for this.

Is it correct to just omit the parameter altogether and change output format?

Jaromir

Le dim. 6 sept. 2020 à 04:41, matthew green  a écrit :
>
> Module Name:src
> Committed By:   mrg
> Date:   Sun Sep  6 02:34:03 UTC 2020
>
> Modified Files:
> src/sbin/amrctl: amrctl.c
>
> Log Message:
> avoid calling printf() %s with NULL.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.11 -r1.12 src/sbin/amrctl/amrctl.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>


Re: CVS commit: src/sys/kern

2020-08-02 Thread Jaromír Doleček
Le dim. 2 août 2020 à 15:57, Taylor R Campbell
 a écrit :
> Why does it improve readability?

Certainly using cringe language features does not help readability.

> What else does -Wvla choke on in src/sys?

Some drm drivers, and uvm, particularly uvm_bio.c. I'd like to work
towards enabling -Wvla in kernel (i.e. remove all VLAs, same as Linux
kernel did in 2018), but I have some other stuff I want to finish
before I pick a new project.

Jaromir


Re: CVS commit: src/sys/kern

2020-08-02 Thread Jaromír Doleček
Readability first and foremost in this case.

I was exploring if I can disable VLAs for the kernel altogether, this
can't be done for now. Nevertheless, this change looked like it would
be useful to make anyway.

Le dim. 2 août 2020 à 01:15, Taylor R Campbell
 a écrit :
>
> > Module Name:src
> > Committed By:   jdolecek
> > Date:   Sat Aug  1 11:18:26 UTC 2020
> >
> > Modified Files:
> > src/sys/kern: subr_autoconf.c
> >
> > Log Message:
> > avoid VLA for the sizeof() calculations
>
> Why?


Re: CVS commit: src/sys/dev/pci

2020-07-17 Thread Jaromír Doleček
One of the things which need to be done is calling the if_ioctl always
with the IFNET_LOCK() held. Right now it sometimes is, and other times
it is not, so it's not possible to rely on it and KASSERT().

As for bnx(4) I did just some basic fixes around making it work with
MSI(-X), since I don't really have easy access to the hw for testing.
This might change soon, then I might look into making it NET_MPSAFE,
after some other bug fixes.

Jaromir

Le sam. 18 juil. 2020 à 00:54, Jason Thorpe  a écrit :
>
>
>
> > On Jul 17, 2020, at 3:50 PM, matthew green  wrote:
> >
> > any chance you can look at NET_MPSAFE here etc? :)
>
> I have a bunch of local changes for this in one of my trees, and I hope to 
> get back to it after netbsd-10 branches.
>
> -- thorpej
>


Re: CVS commit: src/sys/arch/powerpc/include

2020-06-27 Thread Jaromír Doleček
Perhaps we can use a similar technique for mips too? There also the
kernel actually always uses a compile-time fixed page size AFAICS.

Jaromir

Le sam. 27 juin 2020 à 04:51, Rin Okuyama  a écrit :
>
> Module Name:src
> Committed By:   rin
> Date:   Sat Jun 27 02:51:23 UTC 2020
>
> Modified Files:
> src/sys/arch/powerpc/include: vmparam.h
>
> Log Message:
> Restrict {MIN,MAX}_PAGE_SIZE for MODULAR || _MODULE, which makes
> non-MODULAR kernel a little bit efficient.
>
> They are also exposed to userland for jemalloc.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.22 -r1.23 src/sys/arch/powerpc/include/vmparam.h
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>


Re: CVS commit: src/sys/external/bsd/drm2/dist/drm/nouveau

2020-02-13 Thread Jaromír Doleček
The static variable was in #ifdef __NetBSD__ part, so I assumed it
doesn't influence the merge.

Christos already reverted the fallthrough warning fix.

Jaromir

Le jeu. 13 févr. 2020 à 09:18, matthew green  a écrit :
>
> "Jaromir Dolecek" writes:
> > Module Name:  src
> > Committed By: jdolecek
> > Date: Wed Feb 12 20:31:46 UTC 2020
> >
> > Modified Files:
> >   src/sys/external/bsd/drm2/dist/drm/nouveau: nouveau_fbcon.c
> >
> > Log Message:
> > remove superfluous static variable used only to zero attach args
>
> is this necessary for 3rd party code?
>
> please try to avoid changing these for cosmetic reasons.
>
> same for warned code that is forced to warning not error.
> the reason we/i didn't change them is to avoid making
> future drm updates harder -- they're already *really* hard.
>
> thanks.
>
>
> .mrg.


Re: CVS commit: src/sys/uvm

2018-12-10 Thread Jaromír Doleček
Good point, added asserts in genfs_io.c for this.
Le dim. 9 déc. 2018 à 23:56, matthew green  a écrit :
>
> "Jaromir Dolecek" writes:
> > Module Name:  src
> > Committed By: jdolecek
> > Date: Sun Dec  9 20:45:37 UTC 2018
> >
> > Modified Files:
> >   src/sys/uvm: uvm_bio.c
> >
> > Log Message:
> > for direct map case, avoid PGO_NOBLOCKALLOC when writing, it makes
> > genfs_getpages() return unallocated pages using the zero page and
> > PG_RDONLY; the old code relied on fault logic to get it allocated, which
> > the direct case can't rely on
> >
> > instead just allocate the blocks right away; pass PGO_JOURNALLOCKED
> > so that code wouldn't try to take wapbl lock, this code path is called
> > with it already held
>
> can you assert this if not already done?  that the locking is OK,
> i mean...
>
> thanks.
>
>
> .mrg.


Re: CVS commit: src/sys/arch/x86

2018-12-07 Thread Jaromír Doleček
Maybe I missed something earlier - does KASLR being enabled by default
mean that x86 now doesn't any more use the direct map to copy memory
pages?

Jaromir
Le ven. 7 déc. 2018 à 16:47, Maxime Villard  a écrit :
>
> Module Name:src
> Committed By:   maxv
> Date:   Fri Dec  7 15:47:11 UTC 2018
>
> Modified Files:
> src/sys/arch/x86/conf: files.x86
> src/sys/arch/x86/x86: pmap.c
>
> Log Message:
> Add an option to have a static kernel memory layout. This option is
> disabled by default - that is to say, KASLR remains enabled by default.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.103 -r1.104 src/sys/arch/x86/conf/files.x86
> cvs rdiff -u -r1.312 -r1.313 src/sys/arch/x86/x86/pmap.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>


Re: CVS commit: src/sys/arch/x86/x86

2018-07-08 Thread Jaromír Doleček
Shouldn't this:

memtop |= (uint16_t)mpbios_page[0x414] << 8;

be actually << 16 to keep the same semantics?

Jaromir
Le dim. 8 juil. 2018 à 01:05, Kamil Rytarowski  a écrit :
>
> Module Name:src
> Committed By:   kamil
> Date:   Sat Jul  7 23:05:50 UTC 2018
>
> Modified Files:
> src/sys/arch/x86/x86: mpbios.c
>
> Log Message:
> Remove unaligned access to mpbios_page[]
>
> Replace unaligned pointer dereference with a more portable construct that
> is free from Undefined Behavior semantics.
>
> sys/arch/x86/x86/mpbios.c:308:11, load of misaligned address 
> 0x800031c7a413 for type 'const __uint16_t' which requires 2 byte alignment
>
> Detected with Kernel Undefined Behavior Sanitizer
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.66 -r1.67 src/sys/arch/x86/x86/mpbios.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>


Re: CVS commit: src/sys/arch

2018-06-20 Thread Jaromír Doleček
2018-06-20 18:03 GMT+02:00 Maxime Villard :
> I haven't tested but looking at the code it seems to me that this part of
> your
> change breaks DAZ on native, and I don't see how it can help xsave on xen by
> the way

Thanks, I'll change the conditional to avoid any potential change of
behaviour for native.

Yes, I know that fxsave is different than xsave, but on Xen it seems
to be somehow connected.

The conditional on XSAVE wad been necessary on Xen according to my
testing - with no-xsave that function faulted, with xsave active it
didn't.

The only difference in CPUID for working and faulting code was the
OSXSAVE bit. Maybe Xen somehow traps/disables fxsave also with
no-xsave, or maybe requires bigger alignment for some reason. I'll
check if  I can find something to make the fxsave case work for Xen.

Jaromir


Re: CVS commit: src/sys/arch

2018-06-20 Thread Jaromír Doleček
The function is ever called only with x86_fpu_save >= FPU_SAVE_FXSAVE,
so there should be no change in functionality AFAICS.

2018-06-20 10:34 GMT+02:00 Maxime Villard :
> Le 19/06/2018 à 21:50, Jaromir Dolecek a écrit :
>>
>> Module Name:src
>> Committed By:   jdolecek
>> Date:   Tue Jun 19 19:50:19 UTC 2018
>>
>> Modified Files:
>> src/sys/arch/x86/include: fpu.h
>> src/sys/arch/x86/x86: cpu.c fpu.c identcpu.c
>> src/sys/arch/xen/x86: cpu.c
>>
>> Log Message:
>> fix FPU initialization on Xen to allow e.g. AVX when supported by
>> hardware;
>> only use XSAVE when the the CPUID OSXSAVE bit is set, as this seems to be
>> reliable indication
>>
>> tested with Xen 4.2.6 DOM0/DOMU on Intel CPU, without and with no-xsave
>> flag,
>> so should work also on those AMD CPUs, which have XSAVE disabled by
>> default;
>> also tested with Xen DOM0 4.8.3
>>
>> fixes PR kern/50332 by Torbjorn Granlund; sorry it took three years to
>> address
>>
>> XXX pullup netbsd-8
>
>
> I don't understand, and it looks wrong.
>
>  -- fxsave
>
> fpuinit_mxcsr_mask calls fxsave, not xsave. The availability of fxsave is
> controlled by CR4_OSFXSR, not CR4_OSXSAVE. It seems to me that the check you
> added in fpuinit_mxcsr_mask is wrong, because it is confusing fxsave and
> xsave.
>
> [x86_fpu_save = FPU_SAVE_FXSAVE] guarantees fxsave is supported.
>
> fpuinit_mxcsr_mask is called only when this condition is true, so there is
> no
> need for a check in the first place.
>
> The reason I disabled the code on Xen, if I remember correctly, was because
> of a stack alignment problem. For some reason this variable
>
> union savefpu fpusave __aligned(16);
>
> was not always 16-byte aligned on Xen. So fxsave would fault.
>
>  -- xsave
>
> Please keep the initialization logic consistent.
>
> xen/x86/cpu.c
> 554 if (cpu_feature[1] & CPUID2_OSXSAVE)
> 555 cr4 |= CR4_OSXSAVE;
> 556 else {
> 557 x86_xsave_features = 0;
> 558 x86_fpu_save = FPU_SAVE_FXSAVE;
> 559 }
>
> If the availability of xsave on xen is determined by CPUID2_OSXSAVE (as
> opposed to CPUID2_XSAVE on native), then put the code in identcpu.c just
> like native.
>
> That is to say
>
> x86/x86/identcpu.c
> 804 /* See if xsave (for AVX) is supported */
> 805 if ((ci->ci_feat_val[1] & CPUID2_XSAVE) == 0)
> 806 return;
> XXX
> XXX #ifdef XEN
> XXX /* On Xen we also need to check CPUID2_OSXSAVE */
> XXX if ((ci->ci_feat_val[1] & CPUID2_OSXSAVE) == 0)
> XXX return;
> XXX #endif
>
> Note also that your change seems wrong on i386 too, because you enable
> fxsave
> if we don't have xsave, while we may have neither.


Re: CVS commit: src/sys

2018-05-25 Thread Jaromír Doleček
2018-05-22 12:18 GMT+02:00 Martin Husemann :
> Here are timing results:

While 37% speedup is nice, I see there isn't nearly as huge difference
as for my machine. Could you please try with some file which fits
whole in the page cache?

> Unfortunatley with ubc_direct enabled, it panics quickly:

There is a comment in uvm_bio.c about some platforms not being able to
write to individual bytes atomically, and hence software having to do
read/modify/write. Could this be  somehow causing the machine check on
Alpha?

Jaromir


Re: CVS commit: src/sys

2018-05-21 Thread Jaromír Doleček
The very basic test is to do some moral equivalent of "dd if=foo
of=/dev/null bs=64k" with some reasonably sized foo file, with default
setting twice (once to test reading off the disk, second off cache),
then set ubc_direct to 1 via DDB, remount the filesystem and re-run.
Also would be good to check that the transferred contents are correct,
e.g. via sha1.

If that doesn't trigger any panic, further test would be to run some
non-trivial build with ubc_direct set to 1.

Jaromir

2018-05-21 9:11 GMT+02:00 Martin Husemann :
> On Sun, May 20, 2018 at 08:40:23AM -0700, Jason Thorpe wrote:
>> FWIW, I checked in the Alpha changes.  They were trivial.  SH3 and
>> MIPS I didn?t do because there?s extra work to do to handle virtually
>> indexed caches.  (In the case of SH3, it?s actually the SH4 that has
>> the issue, and I do have a Dreamcast, so I may get to that one
>> eventually...)
>
> I can test alpha - is there something specical required for testing?
>
> Martin


Re: regnsub, regasub

2018-02-26 Thread Jaromír Doleček
+1 to removal

2018-02-26 18:51 GMT+01:00 :
>
> Any chance we can remove this from libc before releasing 8.0?
> it has one user, and the implementation is very specific for a certain
> use-case.
>
> having a DIY one use case function in libc is actually harming the
> ability to upstream this, aside from the implementation choices.


Re: CVS commit: src/sys/dev/acpi

2017-12-11 Thread Jaromír Doleček
Can we have acpi_intr_establish() accept the xname? It's very useful to
have the driver name registered for "intrctl list".

Jaromir

2017-12-10 17:51 GMT+01:00 Manuel Bouyer :

> Module Name:src
> Committed By:   bouyer
> Date:   Sun Dec 10 16:51:30 UTC 2017
>
> Modified Files:
> src/sys/dev/acpi: acpi_util.c files.acpi
> Added Files:
> src/sys/dev/acpi: acpi_i2c.c acpi_i2c.h acpi_intr.h
>
> Log Message:
> Implement a ACPI helper to fill the property array expected from our I2C
> framework from the ACPI tables.
> Also implement acpi_intr_establish(), acpi_intr_disestablish() and
> acpi_intr_string().
> Needed for the upcoming HID over I2C support, proposed on tech-kern@
> on Dec, 1.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r0 -r1.1 src/sys/dev/acpi/acpi_i2c.c \
> src/sys/dev/acpi/acpi_i2c.h src/sys/dev/acpi/acpi_intr.h
> cvs rdiff -u -r1.8 -r1.9 src/sys/dev/acpi/acpi_util.c
> cvs rdiff -u -r1.99 -r1.100 src/sys/dev/acpi/files.acpi
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
>


Re: CVS commit: src/games/fortune/datfiles

2017-11-19 Thread Jaromír Doleček
I'd ask you to revert this nonsense.

Jaromir

2017-11-18 21:48 GMT+01:00 Maya Rashish :

> Module Name:src
> Committed By:   maya
> Date:   Sat Nov 18 20:48:50 UTC 2017
>
> Modified Files:
> src/games/fortune/datfiles: fortunes fortunes2 fortunes2-o.real
>
> Log Message:
> Remove a few offensive quotes, put in as many new quotes.
>
> PR bin/52735
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.65 -r1.66 src/games/fortune/datfiles/fortunes
> cvs rdiff -u -r1.59 -r1.60 src/games/fortune/datfiles/fortunes2
> cvs rdiff -u -r1.15 -r1.16 src/games/fortune/datfiles/fortunes2-o.real
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
>


Re: CVS commit: [jdolecek-ncq] src/sys/dev/ata

2017-06-17 Thread Jaromír Doleček
2017-06-17 20:00 GMT+02:00 Jonathan A. Kollasch :
>
> I really think we need to make ata_xfer_init() private to
> ata_queue_alloc(), and use ata_get_xfer() everywhere.  That we can
> fabricate an xfer on the stack anywhere that will trample all over an
> active command slot is fundamentally wrong.

Any non-NCQ command which goes via atastart() is safe, since that routine
makes sure there is only one non-NCQ command being executed by HBA.

The on-stack xfer was carried over from previous design. I kept it mostly
since I didn't want to change those codepaths to deal with xfer not being
available.

As far as I see, ahcisata(4) reset should be actually fine, since it does
the reset
without using the xfers/slots.

I had a look at siisata_reset_drive(). That thing indeed can't reliably
work as-is.
Arguably it should be changed to use the on-stack xfer, currently it e.g.
doesn't
do anything if all slots are taken, which is wrong. I'll have a look.

> ata_queue_downsize() currently appears to leak xfers.  Additionally once
> you put a QD1 drive on the channel you're stuck there forever, even if
> you replace the channel population entirely with QD32 drives.

Okay, I need to look at this - original assumption was that it's never
called while there are other transfers. This is false with PMP.

Jaromir


Re: CVS commit: src/sys/arch/x86/x86

2017-05-31 Thread Jaromír Doleček
You rock! Thank you.

2017-05-31 2:19 GMT+02:00 Maya Rashish :

> Module Name:src
> Committed By:   maya
> Date:   Wed May 31 00:19:17 UTC 2017
>
> Modified Files:
> src/sys/arch/x86/x86: cpu.c
>
> Log Message:
> Do not pause many times between testing if the CPU can go.
>
> This only impacts QEMU as QEMU's implementation of pause is
> significantly slower than its implementation of nop.
>
> PR kern/51623: running qemu-x86_64 with -smp 4 - the additional
> CPUs don't start.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.125 -r1.126 src/sys/arch/x86/x86/cpu.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
>


Re: CVS commit: src/sys

2016-11-11 Thread Jaromír Doleček
2016-11-11 11:52 GMT+01:00 J. Hannken-Illjes :
> Why do we need a TAILQ here, the number of deletions after the first
> element is nearly zero and the list is quite short.

The element would be somewhere towards the end of the list, but likely
not the very last one. Yes, the list is usually short (IIRC the limit
is around 60 per 1MB of log / 1GB of filesystem), so it might not
matter much if we have the error path O(n).

> +wapbl_deallocation_free(struct wapbl *wl, struct wapbl_dealloc *wd,
> +   bool locked)
>
> Better to let the caller always take/release the lock.

I wanted to let the pool_put() be called without the lock, but this
might be not very useful optimisation.

> Returning a pointer to an arbitrary list element and using it
> later is bad design.  Would be better to define as:
>
> wapbl_unregister_deallocation(struct wapbl *wl, daddr_t blk, int len)
> {

I simply want to have it O(1). I think it is useful to keep the error
path there fast, as it would be quite common for wapbl case. Also just
passing the (opaque) pointer makes it simpler.

Thanks for the BAP_ASSIGN() fix. I started investigating only to later
notice you already committed fix.

What you think about removing the NULL initialization, to let compiler
trigger warning there? Seems modern gcc is intelligent enough to
correctly trigger warning, even with the conditional assignment. Of
course it might be not very useful, as hopefully this code won't be
touched again for a long time.

--- ffs_inode.c 11 Nov 2016 10:50:16 -  1.123
+++ ffs_inode.c 11 Nov 2016 20:47:04 -
@@ -583,8 +583,8 @@ ffs_indirtrunc(struct inode *ip, daddr_t
int i;
struct buf *bp;
struct fs *fs = ip->i_fs;
-   int32_t *bap1 = NULL;
-   int64_t *bap2 = NULL;
+   int32_t *bap1;
+   int64_t *bap2;
struct vnode *vp;
daddr_t nb, nlbn, last;
char *copy = NULL;

Jaromir


Re: CVS commit: src/sys

2016-11-07 Thread Jaromír Doleček
2016-11-07 12:11 GMT+01:00 J. Hannken-Illjes :
> It gets used when a block of block pointers has been deallocated and
> brelsed, that is NOT written to disk.  If we allow the deallocation of
> this block to fail and ufs_truncate_retry() runs ffs_truncate again
> it will read the block from disk and free already freed blocks.

Good point. The same is however true for ALL the calls in the chain,
so then we should have to really use FORCE everywhere.

One solution is to always bwrite() or bdwrite() back even fully zeroed
block for wapbl case instead of brelse(BC_INVAL). I think that for
fsck to reliably recover from crash within truncate, this might
actually be needed also for !wapbl case.

Another way how to solve this would be to try to register the
deallocation and on error bail out, before the diving. It would
require cancelling the registration if the diving call returns EAGAIN
however.

> You mean this:
>
> error = ffs_indirtrunc(ip, nlbn, FFS_FSBTODB(fs, nb),
> (daddr_t)-1, level - 1, countp);
> -   if (error)
> -   goto out;
> +   if (error == EAGAIN)
> +   goto out;
> +   else if (error && !allerror)
> +   allerror = error;

No, I mean the copy logic and big blocks with condition on
ip->i_ump->um_mountp->mnt_wapbl.

> I don't understand ... do you want to split into two diffs?

I prefer to split commits into incremental changes if reasonably
possible, so it's easier to review and revise. That's all. That's why
I prefer the fix for immediate corruption to go separately.

Jaromir


Re: CVS commit: src/sys

2016-11-07 Thread Jaromír Doleček
2016-11-07 10:25 GMT+01:00 J. Hannken-Illjes :
> The first part should not be necessary.  After the loop we should have
> "i == last" -- from a quick look "i < last" is impossible.

Yes, I know - it's just to make the diff vs 1.117 smaller and hence
easier to review. I want to commit this separately.

> The second part is right and responsible for most of the panics.

Right.

> Your patch still doesn't address my second observation, if the
> machine crashs inside ffs_truncate we leave the file system in a
> state fsck can't handle.  The blocks we freed get attached to other
> nodes before they get cleared from our on-disk copy leading to
> duplicate blocks.

That patch is not really very ideal.

One is UFS_WAPBL_REGISTER_DEALLOCATION_FORCE(), which should only be
used as desperate measure.

Second is that the patch adds IMO too much difference for wapbl and
!wapbl case, to already quite complicated function. Also it's slightly
confusing that the !wapbl change now doesn't support failure in the
middle any more - it will just leave the block inconsistent. I need to
think a little about this, maybe there is some better way.

Anyway, IMO eventual change to allow fsck to DTRT after crash in
ffs_truncate() should go separately from the crash fix.

Jaromir


Re: CVS commit: src/sys

2016-11-06 Thread Jaromír Doleček
> On Fri, Nov 04, 2016 at 04:44:10PM +0100, J. Hannken-Illjes wrote:
>> - This change results in "panic: ffs_blkfree_common: freeing free block"
>>   if I put a file system under stress (*1).
>>
>> - I suppose not zeroing the blocks to be freed before freeing them
>>   makes the life of fsck harder.
>>
>> - Running "brelse(bp, BC_INVAL)" doesn't look OK.

The brelse(bp, BC_INVAL) call was there before as well, but the
condition changed and is wrong.

I can repeat the problem with your script and the packaged fsx (thanks
Thomas). Whipped up a patch to what looked wrong there, and it no
longer panics for me. Patch is attached. I'll test further and commit
it tomorrow.

Jaromir
Index: ffs_inode.c
===
RCS file: /cvsroot/src/sys/ufs/ffs/ffs_inode.c,v
retrieving revision 1.118
diff -u -p -r1.118 ffs_inode.c
--- ffs_inode.c 28 Oct 2016 20:38:12 -  1.118
+++ ffs_inode.c 6 Nov 2016 23:09:15 -
@@ -675,18 +675,18 @@ ffs_indirtrunc(struct inode *ip, daddr_t
 * Recursively free blocks on the now last partial indirect block.
 */
if (level > SINGLE && lastbn >= 0) {
-   nb = RBAP(ip, last);
+   last = lastbn % factor;
+   nb = RBAP(ip, i);
if (nb != 0) {
error = ffs_indirtrunc(ip, nlbn, FFS_FSBTODB(fs, nb),
-  lastbn % factor, level - 1,
-  countp);
+  last, level - 1, countp);
if (error)
goto out;
}
}
 
 out:
-   if (RBAP(ip, 0) == 0) {
+   if (lastbn < 0 && error == 0) {
/* all freed, release without writing back */
brelse(bp, BC_INVAL);
} else {


Re: CVS commit: src/sys/arch/x86

2016-10-16 Thread Jaromír Doleček
I see Robert added hack meanwhile, I'll look at proper fix tomorrow.

Jaromir

2016-10-16 0:23 GMT+02:00 Joerg Sonnenberger :
> On Sat, Oct 15, 2016 at 04:46:15PM +, Jaromir Dolecek wrote:
>> Module Name:  src
>> Committed By: jdolecek
>> Date: Sat Oct 15 16:46:14 UTC 2016
>>
>> Modified Files:
>>   src/sys/arch/x86/acpi: acpi_machdep.c
>>   src/sys/arch/x86/include: isa_machdep.h
>>   src/sys/arch/x86/isa: isa_machdep.c
>>   src/sys/arch/x86/pci: pciide_machdep.c
>>
>> Log Message:
>> provide intr xname
>
> You broke the Xen kernels on AMD64. Xen's machine/intr.h version is
> different.
>
> Joerg