Re: svn commit: r367066 - head/sys/compat/linux

[Edward Tomasz Napierała]

On 1026T1803, Mateusz Guzik wrote:
> Author: mjg
> Date: Mon Oct 26 18:03:50 2020
> New Revision: 367066
> URL: https://svnweb.freebsd.org/changeset/base/367066
> 
> Log:
>   linux: silence renameat2 flags warning
>   
>   Hogs the console while building the Linux kernel in a Ubuntu Focal jail.

That's what 'compat.linux.debug' sysctl is for.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r365643 - head/bin/cp

[Edward Tomasz Napierała]

> On 25 Sep 2020, at 19:12, Ian Lepore  wrote:

[..]

> (A question that occurs to me:  could it be that the files you've seen
> got created at shutdown after devfs was unmounted, rather than at
> startup?  I don't know enough about the shutdown sequence to know
> whether that's possible.)

Thing is, if you unmount /dev, you are revoking
all the device nodes, including your ttys and disk device nodes.  You wouldn’t 
be able to properly shutdown afterwards.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r362769 - in head/sys: amd64/linux amd64/linux32 arm64/linux compat/linux i386/linux

[Edward Tomasz Napierała]

On 0629T1242, Kyle Evans wrote:
> On Mon, Jun 29, 2020 at 10:27 AM Shawn Webb  
> wrote:
> >
> > Hey Kyle,
> >
> > On Mon, Jun 29, 2020 at 03:09:14AM +, Kyle Evans wrote:
> > > Author: kevans
> > > Date: Mon Jun 29 03:09:14 2020
> > > New Revision: 362769
> > > URL: https://svnweb.freebsd.org/changeset/base/362769
> > >
> > > Log:
> > >   linuxolator: implement memfd_create syscall
> > >
> > >   This effectively mirrors our libc implementation, but with minor 
> > > fudging --
> > >   name needs to be copied in from userspace, so we just copy it straight 
> > > into
> > >   stack-allocated memfd_name into the correct position rather than 
> > > allocating
> > >   memory that needs to be cleaned up.
> > >
> > >   The sealing-related fcntl(2) commands, F_GET_SEALS and F_ADD_SEALS, have
> > >   also been implemented now that we support them.
> > >
> > >   Note that this implementation is still not quite at feature parity 
> > > w.r.t.
> > >   the actual Linux version; some caveats, from my foggy memory:
> > >
> > >   - Need to implement SHM_GROW_ON_WRITE, default for memfd (in progress)
> > >   - LTP wants the memfd name exposed to fdescfs
> > >   - Linux allows open() of an fdescfs fd with O_TRUNC to truncate after 
> > > dup.
> > > (?)
> > >
> > >   Interested parties can install and run LTP from ports (devel/linux-ltp) 
> > > to
> > >   confirm any fixes.
> > >
> > >   PR: 240874
> > >   Reviewed by:kib, trasz
> > >   Differential Revision:  https://reviews.freebsd.org/D21845
> >
> > RELNOTES?
> >
> > >
> > > Modified:
> > >   head/sys/amd64/linux/linux_dummy.c
> > >   head/sys/amd64/linux32/linux32_dummy.c
> > >   head/sys/arm64/linux/linux_dummy.c
> > >   head/sys/compat/linux/linux.c
> > >   head/sys/compat/linux/linux.h
> > >   head/sys/compat/linux/linux_file.c
> > >   head/sys/compat/linux/linux_file.h
> > >   head/sys/i386/linux/linux_dummy.c
> >
> > Should __FreeBSD_version be bumped?
> >
> 
> I'm roping in trasz@, because I'm unsure on either of these points --
> I haven't paid attention and don't know if we typically include linux
> syscalls that we implement in relnotes, and given that this commit
> only really affects pre-compiled Linux binaries I'm not sure if
> there's any utility in bumping __FreeBSD_version; presumably ports
> folks can't do anything differently here, and binaries will work just
> the same.

I don't think we need to bump the version here.  As for the relnotes:
I hadn't considered that before, but it sounds like a good idea; we
probably do want to at least enumerate the major Linuxulator changes
there.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r354458 - head/libexec/rc/rc.d

[Edward Tomasz Napierała]

On T2115, Alexander Leidinger wrote:
> Quoting Edward Tomasz Napierała  (from Sun, 10 Nov  
> 2019 20:25:04 +):
> 
> > On 1110T1147, Conrad Meyer wrote:
> 
> >> I am on board with making this stuff more “batteries included” and usable
> >> by default, but just echoing the request for configurable knobs (I don’t
> >> care about the defaults).
> >
> > My point is... well, there are two.  First is, it's configurable already:
> > you can comment out parts of the rc script you don't want, or you can not
> 
> We want to have the system rc scripts "no touch" scripts, don't we?

I don't quite agree, but I'm not going to fight for it.

> > set linux_enable in the first place and do things it would otherwise do
> > for you by in the usual manner - add the modules to loader.conf, add a
> > line to sysctl.conf etc.  The script simply provides a shortcut to match
> > what 90% of users want.
> 
> I agree.
> 
> > Second, in order to implement something properly, I need to understand
> > how it's going to be used.  I guess I worded it quite badly in the
> > previous mail; it's wasn't supposed to sound like "Can you give me some
> > use case because I think you're wrong", but rather a "Can you give me
> > some use case, because without it I have no idea how to design it to
> > fit it".
> 
> What about a config variable which enables or disables the mounts?  
> That way we don't have to modify the rc script to get back to the old  
> behavior in cases were we need it.

Okay, this is easy to do; see https://reviews.freebsd.org/D22320.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r354458 - head/libexec/rc/rc.d

[Edward Tomasz Napierała]

On 1110T1147, Conrad Meyer wrote:
> Hi,
> 
> Response inline below.
> 
> On Sun, Nov 10, 2019 at 08:08 Edward Tomasz Napierala 
> wrote:
> 
> > On 1109T2049, Alexander Leidinger wrote:
> > > Quoting Edward Tomasz Napierala  (from Thu, 7 Nov
> > > 2019 18:15:24 + (UTC)):
> > >
> > > > Author: trasz
> > > > Date: Thu Nov  7 18:15:24 2019
> > > > New Revision: 354458
> > > > URL: https://svnweb.freebsd.org/changeset/base/354458
> > > >
> > > > Log:
> > > >   Extend the linux rc script to mount the neccessary file systems,
> > > >   set ELF fallback brand, and load pty(4).
> > >
> > > We never did something like that. We have it documented everywhere
> > > that it needs to be done manually. So this is at least a POLA
> > > violation. It is great that the nocover option is used in the mount,
> > > but it's still some kind of layering violation (I may want to have
> > > only a subset mounted, or nothing at all).
> >
> > It is kind of a POLA violation, but previously the linux_enable
> > knob didn't do much apart from loading the linux{,64}.ko kernel
> > module and doing something weird with ldconfig, which I'm not
> > even sure is actually useful.  In other words, the old behaviour
> > can be restored by just not using linux_enable, and instead
> > loading the kernel modules the same way all the others are loaded.
> >
> > Could you give me some use case why someone would want only a subset
> > of the filesystems?
> 
> 
> They’re an additional attack surface.  If your few linux applications get
> by with fewer vfs, you might want to avoid the others. I’m not particularly
> attached to this reason. And imo, linux64.ko kind of dwarf that attack
> surface concern, so it’s maybe a silly point.

Yeah.  Imho it's kind of like removing parts of kernel itself - you
can do that, if you really wanted to, but we don't provide separate
knobs to make it particularly easy.

> Another problem with the current code is (and I may be mistaken here) I
> think that it ignores mount options configured in the admin’s /etc/fstab.
> Eg I configure my /tmp with a hard limit on memory use and if I were to
> mount a compat shm tmpfs, I’d also want its memory use bounded. “Nocover”
> protects from covering any existing “auto“ mounts, but one can imagine
> specifying the linux compat mount “noauto”.
> 
> I am on board with making this stuff more “batteries included” and usable
> by default, but just echoing the request for configurable knobs (I don’t
> care about the defaults).

My point is... well, there are two.  First is, it's configurable already:
you can comment out parts of the rc script you don't want, or you can not
set linux_enable in the first place and do things it would otherwise do
for you by in the usual manner - add the modules to loader.conf, add a
line to sysctl.conf etc.  The script simply provides a shortcut to match
what 90% of users want.

Second, in order to implement something properly, I need to understand
how it's going to be used.  I guess I worded it quite badly in the
previous mail; it's wasn't supposed to sound like "Can you give me some
use case because I think you're wrong", but rather a "Can you give me
some use case, because without it I have no idea how to design it to
fit it".

[..]

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r343416 - head/bin/sh

[Edward Tomasz Napierała]

On 0125T1813, Alexey Dokuchaev wrote:
> On Fri, Jan 25, 2019 at 06:02:58AM +, Edward Napierala wrote:
> > The aliases are gone, let's continue on the remaining bits below.
> 
> They are not gone, they were commented out; also, bogus double linefeeds
> are not gone.

Yeah, I've matched the existing comment style in that file.

> But most importantly, this whole file is useless and IMHO
> should just be removed.  /bin/sh is not supposed to be one's interactive
> shell.

Of course it is an interactive shell.  We've been using it as the
default interactive shell for non-root accounts since... ages.

> > It is alien, because it's different from their experience
> > from other systems they are used to.
> 
> This argument does not really hold because /bin/sh is not... see above.
> 
> > It doesn't affect existing installs.  It doesn't affect people
> > who run the default root shell (tcsh), nor folks who use shells
> 
> AFAICT default root shell is /bin/csh, not tcsh. ;-)  But that also

Our csh _is_ tcsh.

> means that /usr/src/bin/sh/dot.shrc doesn't have to exist: those who
> change the root shell should either pick another interactive shell,
> or if they want /bin/sh be ready to deal with sanitary environment.

See above.

> > And for folks who do have their own tree with their preferred
> > /root/.shrc to "make distribution" from, it should actually make
> > their diff to upstream smaller.
> 
> I don't like extra files, esp. configuration files that look like they
> are for interactive shell while our /bin/sh is in fact not.  This is
> confusing, and FreeBSD is not supposed to be confusing.

Again, sh(1) is an interactive shell.

> > It is a syntax problem:
> > 
> > trasz@v2:~ % while :; do date; sleep 1; done
> > while: Expression Syntax.
> > do: Command not found.
> > done: Command not found.
> 
> Are you trying to use sh(1) loop in (t)csh?  Why?  And what does it have
> to do with the /usr/src/bin/sh/dot.shrc issue?

I'm trying to explain the basic problem with our default shell:
it doesn't work, because it can't handle what people call the
shell syntax.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r340478 - head/share/man/man7

[Edward Tomasz Napierała]

> On 16 Nov 2018, at 18:40, Cy Schubert  wrote:
> 
> In message <201811161804.wagi44wc047...@pdx.rh.cn85.dnsmgr.net>, 
> "Rodney W. Gri
> mes" writes:
 On Fri, Nov 16, 2018 at 7:29 AM Mateusz Piotrowski <0...@freebsd.org> 
 wrote:
 
  A few years ago jilles@ proposed changing reboot's default to signallin
>> g
  init (preserving reboot -q which just invokes the reboot system call),
 but
  this was not accepted. Perhaps this can be tried again for 13.0.
 
>>> 
>>> I didn't like it at the time, however I was wrong. Much of my reasoning for
>>> doing it has become muted as well since then, and the need to do it has
>>> become more amplified as more rc scripts have grown shutdown
>>> functionality...
>>> 
>>> I think if we make what's now reboot 'fastreboot' or 'reboot -q' (both of
>>> which are historic replacements), we can make 'reboot' what's now 'shutdown
>>> -r now'.
>> 
>> I support this position.
> 
> reboot(2) should be changed to signal init(8). RB_AUTOBOOT should 
> signal init while a new RB_FASTBOOT or RB_LEGACY (or pick a name) would 
> preserve traditional behavior. RB_POWEROFF, RB_POWERCYCLE and RB_HALT 
> would also signal init except when RB_FASTBOOT flag is set.

Wouldn’t this break rebooting when UID 1 is not init(8)?

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r334199 - head/usr.sbin/bhyve

[Edward Tomasz Napierała]

On 0526T0226, Marcelo Araujo wrote:
> 2018-05-26 2:21 GMT+08:00 Brooks Davis :

[..]

> > The correct code here would be one of:
> >
> > str = strdup(opt);
> > if (str == NULL)
> > goto out;
> >
> 
> No, it is not the correct code! If we go out and free(str) we have nothing
> to free, because we even didn't allocated memory for str.

FWIW, calling free(3) on a NULL pointer is perfectly fine.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r328013 - head/sbin/fsck_ffs

[Edward Tomasz Napierała]

On 0309T2136, David Bright wrote:
> On Mar 9, 2018, at 17:31, Ian Lepore  wrote:
> > 
> > On Fri, 2018-03-09 at 17:09 -0500, Mark Johnston wrote:
> >> 
> >> etc/rc.d/fsck doesn't know how to interpret the new exit code and now
> >> just drops to a single-user shell when it is encountered. […]
> >> 
> >> Is there any reason etc/rc.d/fsck shouldn't automatically retry (up to
> 
> This is, in fact, the reason that I made the change I did. I was trying to 
> put in a retry loop to rc.d/fsck, but found that I couldn’t get it to work 
> because fsck and fsck_ffs were not exiting with non-zero status. The drop to 
> single user is not really due to the specific (new) error code of 16, it is 
> due to the fact that fsck_ffs is now exiting with a non-zero status when it 
> hasn’t completely cleaned the file system; /any/ non-zero status would cause 
> the current rc.d/fsck script to go to single user. Prior to my change, 
> fsck_ffs was exiting with a zero status even though it had not completely 
> cleaned the filesystem and told the user to run it again.
> 
> > 
> > fsck_ffs already has a -R flag to automatically retry, wouldn't that be
> > a better mechanism for handling this new type of retry?
> 
> That’s true; however, there is currently no way to pass that flag through the 
> filesystem-agnostic fsck wrapper called from rc.d/fsck to the 
> filesystem-specific fsck_ffs program that it calls. One could implement a 
> similar flag on the fsck wrapper to be passed along to the 
> filesystem-specific checker, but I think fsck_ffs is the only one that 
> currently implements such a flag. 

Sure there is.  See /etc/defaults/rc.conf:

fsck_y_flags="-T ffs:-R -T ufs:-R"  # Additional flags for fsck -y

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r325965 - head/libexec/rtld-elf

[Edward Tomasz Napierała]

On 1119T1040, Pedro Giffuni wrote:
> 
> 
> On 11/19/17 07:12, Edward Tomasz Napierala wrote:
> > On 1118T1610, Konstantin Belousov wrote:
> >> On Sat, Nov 18, 2017 at 01:21:22PM +, Edward Tomasz Napierala wrote:
> >>> Author: trasz
> >>> Date: Sat Nov 18 13:21:22 2017
> >>> New Revision: 325965
> >>> URL: https://svnweb.freebsd.org/changeset/base/325965
> >>>
> >>> Log:
> >>>Increase rtld initial memory pool size from 32kB to 128kB.
> >>>
> >>>The old value was probably fine back in 1998, when that code was 
> >>> imported
> >>>(although the comments still mention VAX, which was quite obsolete by 
> >>> then);
> >>>now, however, it's too small to handle our libc, which results in some
> >>>additional calls to munmap/mmap later on.  Asking for more virtual 
> >>> address
> >>>space is virtually free, and syscalls are not, thus the change.
> >>>
> >>>It was suggested by kib@ that this might be a symptom of a deeper 
> >>> problem.
> >>>It doesn't only affect libc, though - the change also improves rtld 
> >>> memory
> >>>management for eg KDE libraries.  I guess it's just a natural bloat.
> >> This is not what I said.
> >>
> >> My guess was that the large allocation you see in the ktrace output as
> >> coming from rtld was really an allocation of the TLS segment, and it was
> >> so large because libc has that large TLS segment. You did not checked this
> >> guess against the actual code.
> > Right, I stand corrected.
> >
> >> If my guess is true, I do not see a point in the change you made:  the
> >> memory consumption is externally imposed on rtld, and we should not try
> >> to tailor it to single, whenever important, consumer.
> > Here's where I disagree.  The rtld is not some abstract concept, it's one
> > of the components of the operating system, and it can and should be tweaked
> > to match real life situations.  Especially when it affects virtually all
> > of its use cases, as is the case with libc.
> >
> >
> 
> Being pragmatic ...
> 
> Determining such values is almost always a trial-and-error process. 
> Perhaps 64k is makes everyone (especially KDE) happy?

It was determined by trial and error.  128k is the smallest one that removes
the need for addidional call to mmap on startup - not for KDE, but for every
binary linked with libc, including true(1).  Running kdeinit4 still results
in a few such calls - although fewer than before (the value also affects the
size of subsequent rtld allocations).

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r324471 - head/sys/boot

[Edward Tomasz Napierała]

On 1013T1438, Devin Teske wrote:
> 
> > On Oct 13, 2017, at 8:53 AM, Ngie Cooper  wrote:
> > 
> > 
> >> On Oct 9, 2017, at 21:57, Ngie Cooper (yaneurabeya) 
> >>  wrote:
> >> 
> >> 
> >>> On Oct 9, 2017, at 21:56, Warner Losh  wrote:
> >>> 
> >>> DO NOT MAKE EDITS TO sys/boot. YOU ARE CREATING CONFLICTS FOR ME.
> >>> 
> >>> DO NOT MAKE ANY COMMITS TO sys/boot.
> >>> 
> >>> BACK OFF.
> >>> 
> >>> Seriously, though, extra changes create extra friction, and these changes 
> >>> aren't worth any friction at all. I'm deleting LIBSAU and this guarantees 
> >>> a conflict when I update.
> >>> 
> >>> So please, do not make any edits to sys/boot whatsoever, no matter how 
> >>> trivial.
> >>> 
> >>> At least until I'm done.
> >> 
> >> Ok
> > 
> > So... what’s the status?
> 
> Your typo gave me an idea for a new tool ...
> 
> Usage: whatfs path ...
> 
> Return unique list of filesystems containing list of paths.
> 
> Lol ;D
> 
> E.g., "whatfs / /usr /usr/local /foo /bar" would list minimal filesystem 
> mountpoints containing all given paths

Like this?

df / /usr /usr/local /foo /bar | awk 'NR > 1 { print $6 }'  | sort -u

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r313854 - head/sys/cam/ctl

[Edward Tomasz Napierała]

On 0217T1522, Alexander Motin wrote:
> On 17.02.2017 12:24, Edward Tomasz Napierała wrote:
> > On 0217T0522, Alexander Motin wrote:
> >> Author: mav
> >> Date: Fri Feb 17 05:22:58 2017
> >> New Revision: 313854
> >> URL: https://svnweb.freebsd.org/changeset/base/313854
> >>
> >> Log:
> >>   Change the way MaxCmdSN is used.
> >>   
> >>   Before this change MaxCmdSN was reported as CmdSN + delta, that made it
> >>   limit number of requests in transmission from the initiator to target,
> >>   that was pretty useless.  After this change MaxCmdSN limits number of
> >>   requests queued to CTL, i.e. maximal queue depth for the initiator.
> >>   The default limit is 256 outstanding requests per initiator at a time.
> >>   
> >>   This code uses existing cs_outstanding_ctl_pdus counter to track queue
> >>   depth.  It's semantics doen't perfectly match, but close enough to not
> >>   add another counter.  Just don't set the maxtags below 2.
> > 
> > I like the change in principle, but I don't like the gratuitous change
> > of sysctl name.  It will break people's configs.  Could you rename it
> > back?  Thanks!
> 
> But its meaning is different now.  Do you know anybody tuning the old
> sysctl?  I can't imagine who would do it and what for.

Yes.  It's been asked about a few times on forums, and it's documented
in ctl(4).  The new meaning is slightly different, sure, but I'd say
it's very close, except that now it really works :-)

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r313854 - head/sys/cam/ctl

[Edward Tomasz Napierała]

On 0217T0522, Alexander Motin wrote:
> Author: mav
> Date: Fri Feb 17 05:22:58 2017
> New Revision: 313854
> URL: https://svnweb.freebsd.org/changeset/base/313854
> 
> Log:
>   Change the way MaxCmdSN is used.
>   
>   Before this change MaxCmdSN was reported as CmdSN + delta, that made it
>   limit number of requests in transmission from the initiator to target,
>   that was pretty useless.  After this change MaxCmdSN limits number of
>   requests queued to CTL, i.e. maximal queue depth for the initiator.
>   The default limit is 256 outstanding requests per initiator at a time.
>   
>   This code uses existing cs_outstanding_ctl_pdus counter to track queue
>   depth.  It's semantics doen't perfectly match, but close enough to not
>   add another counter.  Just don't set the maxtags below 2.

I like the change in principle, but I don't like the gratuitous change
of sysctl name.  It will break people's configs.  Could you rename it
back?  Thanks!

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r313655 - head/sys/vm

[Edward Tomasz Napierała]

On 0211T2027, Konstantin Belousov wrote:
> Author: kib
> Date: Sat Feb 11 20:27:39 2017
> New Revision: 313655
> URL: https://svnweb.freebsd.org/changeset/base/313655
> 
> Log:
>   Change type of the prot parameter for kern_vm_mmap() from vm_prot_t to int.
>   
>   This makes the code to pass whole word of the mmap(2) syscall argument
>   prot to the syscall helper kern_vm_mmap(), which can validate all
>   bits.  The change provides temporal fix for sys/vm/mmap_test
>   mmap__bad_arguments, which was broken after r313352.

Thanks, and sorry for the breakage.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r311283 - head/sys/cam/ctl

[Edward Tomasz Napierała]

Dnia 04.01.2017 o godz. 16:38 Ryan Stone  napisał(a):

>> On Wed, Jan 4, 2017 at 7:50 AM, Edward Tomasz Napierala  
>> wrote:
>> +   refcount_release(>cs_outstanding_ctl_pdus);
> 
> Shouldn't the return value of refcount_release() be checked?

Not in this case.  The destruction of  cfumass sessions is always done in one 
specific place; everywhere else we don't care whether the reference was the 
last one or not.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r310354 - in head/usr.sbin: . prometheus_sysctl_exporter

[Edward Tomasz Napierała]

On 1221T2006, Ed Schouten wrote:
> Hi Adrian,
> 
> 2016-12-21 18:32 GMT+01:00 Adrian Chadd :
> > Maybe just "sysctl_export" would be fine.
> 
> Good point. Almost all Prometheus metrics exporters use this naming scheme:
> 
> ~80%: ${foo}_exporter
> ~20%: prometheus_${foo}_exporter
> 
> Examples;
> 
> - https://github.com/prometheus/node_exporter
> - https://github.com/ewr/elasticsearch_exporter
> - https://github.com/jonnenauha/prometheus_varnish_exporter
> 
> The reason why I went for the 'prometheus_' prefix was mainly because
> of the following totally subjective reasons:
> 
> - I didn't want to wreck tab completion. Right now if you type in
> 'sysc' and press tab, it tends to complete to 'sysctl'.

That's a very good reason; it could also be fixed by moving
it from sbin/ to libexec/.

> - This tool is part of the base system, meaning that it will be
> present even if the user did nothing special. This is why I wanted the
> name to be a bit more self-documenting.
> - Thinking ahead: what if we're going to add exporters for other
> things as well? E.g., for Sendmail, sshd, ftpd, HAST, etc. In that
> case it would be nicer if those exporters shared a common prefix.

Another advantage of having the "prometheus" in the name is that it's
easily googlable, and it clearly indicates the "protocol" (in the wide
meaning) one is supposed to use to talk to it.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r305968 - head/etc/autofs

[Edward Tomasz Napierała]

Good point; fixed.

On 0919T2219, Warner Losh wrote:
> For MSDOS it's one thing and likely helps. But for ufs it doesn't
> help. Soft updates already does an excellent job at reducing wear and
> all async does is give added danger of dataloss. Linux distros don't
> matter for UFS because different design decisions have been made for
> ext[234] and UFS.
> 
> Warner
> 
> On Mon, Sep 19, 2016 at 9:43 PM, Edward Tomasz Napierala
>  wrote:
> > Mounting removable media async improves performance and reduces wear.
> > And AFAIK that's what most Linux distros used to do.
> >
> > On 0919T2032, Ronald Klop wrote:
> >> Hi,
> >>
> >> Your commit message says in words exactly what the diff says in code.
> >> Would you like to elaborate on why the change is made? I'm curious
> >> because this could cause severe damage to the filesystem on unclean eject
> >> of the media.
> >>
> >> Regards,
> >> Ronald.
> >>
> >>
> >> On Mon, 19 Sep 2016 10:51:27 +0200, Edward Tomasz Napierala
> >>  wrote:
> >>
> >> > Author: trasz
> >> > Date: Mon Sep 19 08:51:27 2016
> >> > New Revision: 305968
> >> > URL: https://svnweb.freebsd.org/changeset/base/305968
> >> >
> >> > Log:
> >> >   Make autofs use the "async" flag for msdosfs and ufs filesystems
> >> > mounted
> >> >   on /media.
> >> >  MFC after: 1 month
> >> >
> >> > Modified:
> >> >   head/etc/autofs/special_media
> >> >
> >> > Modified: head/etc/autofs/special_media
> >> > ==
> >> > --- head/etc/autofs/special_media   Mon Sep 19 07:47:56 2016
> >> > (r305967)
> >> > +++ head/etc/autofs/special_media   Mon Sep 19 08:51:27 2016
> >> > (r305968)
> >> > @@ -46,6 +46,8 @@ print_map_entry() {
> >> > "Cannot mount ${_fstype} formatted device 
> >> > /dev/${_p}: Install
> >> > sysutils/fusefs-ntfs first"
> >> > exit 1
> >> > fi
> >> > +   elif [ "${_fstype}" = "msdosfs" -o "${_fstype}" = "ufs" ]; then
> >> > +   echo "-fstype=${_fstype},nosuid,async   :/dev/${_p}"
> >> > else
> >> > echo "-fstype=${_fstype},nosuid :/dev/${_p}"
> >> > fi
> >> > ___
> >> > svn-src-all@freebsd.org mailing list
> >> > https://lists.freebsd.org/mailman/listinfo/svn-src-all
> >> > To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
> >
> 
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r302297 - in head/sys/cddl/contrib/opensolaris: common/acl uts/common/fs/zfs uts/common/sys

[Edward Tomasz Napierała]

On 0630T1455, Alexander Motin wrote:
> Author: mav
> Date: Thu Jun 30 14:55:49 2016
> New Revision: 302297
> URL: https://svnweb.freebsd.org/changeset/base/302297
> 
> Log:
>   Revert r299454 and r299448.
>   
>   Those changes were found confusing FreeBSD libc ACL code, that doesn't
>   differentiate ACL for directories and files, and report ACLs for all
>   directories created after those patches as non-trivial.  On the other
>   side these changes were considered wrong from POSIX and NFSv4 points of
>   view.  Until further investigation done upstream, revert those changes
>   locally in preparation for FreeBSD 11.0 release.

Thanks!  But it still doesn't pass the regression tests, due to broken
(or at least incompatible) interaction between inheritance and umask.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r299448 - in head/sys/cddl/contrib/opensolaris: common/acl uts/common/fs/zfs uts/common/sys

[Edward Tomasz Napierała]

On 0619T1733, Alexander Motin wrote:
> On 19.06.16 17:28, Cy Schubert wrote:
> > In message <20160619080803.GA1638@brick>, Edward Tomasz 
> > =?utf-8?Q?Napiera=C5=82
> > a?= writes:
> >> On 0614T0232, Jan Beich wrote:
> >>> Alexander Motin  writes:
> >>>
>  Author: mav
>  Date: Wed May 11 13:43:20 2016
>  New Revision: 299448
>  URL: https://svnweb.freebsd.org/changeset/base/299448
> 
>  Log:
>    MFV r299442: 6762 POSIX write should imply DELETE_CHILD on directories 
> >> - and
>    some additional considerations
>    
>    Reviewed by: Gordon Ross 
>    Reviewed by: Yuri Pankov 
>    Author: Kevin Crowe 
>    
>    openzfs/openzfs@d316fffc9c361532a482208561bbb614dac7f916
> >>>
> >>> This commit confuses acl_is_trivial_np(3). Notice '+' in ls(1) and 'D'
> >>> in getfacl(1) outputs.
> >>
> >> It's not just that.
> >>
> >> Those changes:
> >>
> >> 1. Confuse acl_is_trivial_np(3), as you say.  It's hard to fix in libc,
> >>because they make trivial ACLs different for files and directories,
> >>and acl_is_trivial_np(3) has no way of telling which is which.
> >>
> >> 2. They make delete deny permission take precedence over the containing
> >>directory write allow permission, which is rather different from what
> >>people expect in unix systems, and is against the NFSv4 specification,
> >>even though it might be a better fit for Windows.
> > 
> > This is Windows behavior and inconsistent with the rest of FreeBSD and any 
> > UNIX or Linux system.
> > 
> >>
> >> 3. They make umask apply to inherit_only permissions, and
> >>
> >> 4. I don't fully understand this one yet, but from the ACL regression
> >>test suite (which lives in tests/sys/acl/, and I'd appreciate people
> >>actually ran this before committing ACL-related changes) it looks
> >>like it makes umask not apply to the stuff it should.
> >>
> >> The #1 could be fixed by making ZFS not setting delete_child on write,
> >> basically reverting to the previous behaviour in that aspect.  As for
> >> the others...  I'm not saying each one of those is wrong, but they
> >> certainly warrant further discussion, especially #2 and #4.
> > 
> > I think #2 is wrong behavior on any UNIX-like or POSIX system.
> > 
> >>
> >> Basically, what I'm trying to say is that we should consider backing
> >> this out for 11.0-RELEASE, reverting to the previous semantics, verified
> >> by passing the regression tests.
> > 
> > Agreed.
> > 
> > What in FreeBSD was this patch supposed to solve in the first place?
> 
> Growing divergence from OpenZFS upstream.  I am not advocating this
> patch, but it would be good, if possible, to not revert it completely,
> but block wrong behavior with some minimal ifdefs to make further ZFS
> merges easier.  Help would be appreciated. ;)

Our family just expanded, and thus I'm afraid I won't be able to help
for the next few weeks.  That's one of the reasons why I've suggested
the backout for 11.0 - not a permanent "let's ignore this piece of code
forever" backout, but a temporary one, for 11.0; we would then go back
to the topic after the release.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r299448 - in head/sys/cddl/contrib/opensolaris: common/acl uts/common/fs/zfs uts/common/sys

[Edward Tomasz Napierała]

On 0614T0232, Jan Beich wrote:
> Alexander Motin  writes:
> 
> > Author: mav
> > Date: Wed May 11 13:43:20 2016
> > New Revision: 299448
> > URL: https://svnweb.freebsd.org/changeset/base/299448
> >
> > Log:
> >   MFV r299442: 6762 POSIX write should imply DELETE_CHILD on directories - 
> > and
> >   some additional considerations
> >   
> >   Reviewed by: Gordon Ross 
> >   Reviewed by: Yuri Pankov 
> >   Author: Kevin Crowe 
> >   
> >   openzfs/openzfs@d316fffc9c361532a482208561bbb614dac7f916
> 
> This commit confuses acl_is_trivial_np(3). Notice '+' in ls(1) and 'D'
> in getfacl(1) outputs.

It's not just that.

Those changes:

1. Confuse acl_is_trivial_np(3), as you say.  It's hard to fix in libc,
   because they make trivial ACLs different for files and directories,
   and acl_is_trivial_np(3) has no way of telling which is which.

2. They make delete deny permission take precedence over the containing
   directory write allow permission, which is rather different from what
   people expect in unix systems, and is against the NFSv4 specification,
   even though it might be a better fit for Windows.

3. They make umask apply to inherit_only permissions, and

4. I don't fully understand this one yet, but from the ACL regression
   test suite (which lives in tests/sys/acl/, and I'd appreciate people
   actually ran this before committing ACL-related changes) it looks
   like it makes umask not apply to the stuff it should.

The #1 could be fixed by making ZFS not setting delete_child on write,
basically reverting to the previous behaviour in that aspect.  As for
the others...  I'm not saying each one of those is wrong, but they
certainly warrant further discussion, especially #2 and #4.

Basically, what I'm trying to say is that we should consider backing
this out for 11.0-RELEASE, reverting to the previous semantics, verified
by passing the regression tests.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r299371 - in head: sbin/camcontrol sys/cam sys/cam/scsi

[Edward Tomasz Napierała]

On 0517T1158, Warner Losh wrote:
> On Tue, May 10, 2016 at 11:33 AM, Edward Tomasz Napierala  > wrote:
> 
> > On 0510T1020, Alan Somers wrote:
> > > On Tue, May 10, 2016 at 9:46 AM, Edward Tomasz Napierala <
> > tr...@freebsd.org>
> > > wrote:
> > >
> > > > Author: trasz
> > > > Date: Tue May 10 15:46:33 2016
> > > > New Revision: 299371
> > > > URL: https://svnweb.freebsd.org/changeset/base/299371
> > > >
> > > > Log:
> > > >   Add "camcontrol reprobe" subcommand, and implement it for da(4).
> > > >   This makes it possible to manually force updating capacity data
> > > >   after the disk got resized. Without it it might be neccessary to
> > > >   reboot before FreeBSD notices updated disk size under eg VMWare.
> > > >
> > > >   Discussed with:   imp@
> > > >   MFC after:1 month
> > > >   Sponsored by: The FreeBSD Foundation
> > > >   Differential Revision:https://reviews.freebsd.org/D6108
> > > >
> > > > Modified:
> > > >   head/sbin/camcontrol/camcontrol.8
> > > >   head/sbin/camcontrol/camcontrol.c
> > > >   head/sys/cam/cam_ccb.h
> > > >   head/sys/cam/cam_xpt.c
> > > >   head/sys/cam/scsi/scsi_da.c
> > > >
> > > >
> > >
> > > I too have been annoyed that "camcontrol rescan" won't update capacity
> > > data.  But could we solve the problem by simply adding logic to
> > "camcontrol
> > > rescan" instead of adding an entirely new command?  Would a user ever
> > want
> > > to rescan a device without reprobing it too?
> >
> > Two reasons.  First, I want to be able to pass the device name (like
> > 'da0') and not the CAM path (like 1:0:0) for usability reasons - it seems
> > easy to figure out the latter from the former, using "camcontrol devlist",
> > but it suddenly becomes complicated when you try to explain it in a man
> > page.
> 
> 
> You can look up one or the other. fwdownload uses the daX name.

Indeed.  But it would mean fixing "camcontrol rescan" first.

> > Second - I don't understand the "camcontrol rescan" logic well
> > enough, and "camcontrol rescan all" sometimes fails for me anyway,
> > in a way I'm not sure how to debug.
> >
> 
> That's a cop-out. CAM is hard, but if you aren't willing to figure itout,
> adding hacks that the other CAM maintainers have to cope with doesn't
> help.

That's true.  However, this hack is pretty non-intrusive - it only adds
a trivial amount of code, and the "reprobe" command could be replaced
with a simple alias to "rescan" if someone steps up to reimplement it.

> Also, to be honest I'm not sure those two are actually that related.
> > Rescanning is about discovering new devices on the bus.  "Reprobe"
> > is about updating... well, mostly updating the capacity.  The former
> > requires enumerating the bus using a mechanism built into XPT; the
> > latter is just notifying the periph driver (in this case da(4)) that
> > it needs to query the capacity and call disk_resize(4).
> >
> 
> The two are very related. Now we have two stupid paths in CAM instead of
> one.

We have two clearly separated code paths, doing completely different
things - one scanning the bus, and only notifying periph drivers if
new device is discovered, and the other one to notify existing periph
driver instances, without scanning anything.  I just don't see how
entangling them with each other would improve things.

> and you didn't do ada like I asked.

As I said in review, the ada(4) driver seems to lack resizing
capability.  It doesn't contain a call to disk_resize(9).  It's been
a few years since I've added resizing to da(4), but it took quite 
some time to make sure it interfaces with existing code in exactly
the right way.  I just don't have time for this kind of side quest
right now.  And I'm not even sure how to test it.  With da(4) it
was easy - I've just added LUN resizing to CTL.

> Not happy with this at all, but not asking for a back out.

Thanks.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r296548 - in head/sys: dev/agp dev/drm2 dev/drm2/i915 modules/drm2/i915kms

[Edward Tomasz Napierała]

I have the exact same problem on T420.  Switching consoles back and forth
works as a workaround.

On 0411T1633, Adrian Chadd wrote:
> hiya,
> 
> My x230 backlight doesn't come back on after I suspend/resume :( Any
> ideas? does it work fine for you?
> 
> 
> 
> -adrian
> 
> 
> On 9 March 2016 at 17:31, Conrad Meyer  wrote:
> > On Wed, Mar 9, 2016 at 12:48 PM, Adrian Chadd  
> > wrote:
> >> Woo!
> >>
> >> Just so its' not lost - people in irc have found power consumption has
> >> jumped dramatically since this commit. :(
> >
> > For the record, on X230 (Ivybridge) I actually see lower power
> > consumption with the new 3.8 kms:
> >
> > (All at brightness 100)
> > Old kernel, no KMS:  11W
> > Old kernel, KMS:  13W
> > New kernel, no KMS:  11W
> > New kernel, KMS:  11W
> >
> > With brightness down to minimum (5%):
> > New kernel, KMS:  8.9W
> >
> > Best,
> > Conrad
> 
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r297190 - head/sys/kern

[Edward Tomasz Napierała]

On 0325T0809, Bruce Evans wrote:
> On Thu, 24 Mar 2016, Edward Tomasz [utf-8] Napiera�~Ba wrote:
> 
> > On 0324T1015, Warner Losh wrote:
> >> On Thu, Mar 24, 2016 at 9:46 AM, Ian Lepore <i...@freebsd.org> wrote:
> >>
> >>> On Thu, 2016-03-24 at 16:01 +0100, Edward Tomasz Napierała wrote:
> >>>> On 0324T1609, Alexander Motin wrote:
> >>>>> On 24.03.16 15:42, Edward Tomasz Napierała wrote:
> >>>>>> On 0324T1032, Jean-Sébastien Pédron wrote:
> >>>>>>> On 23/03/2016 18:45, Edward Tomasz Napierala wrote:
> >>>>>>>>> So maybe callouts are disabled in this situation. If there
> >>>>>>>>> is a way to
> >>>>>>>>> detect that, then vt(4) can go back to a "synchronous mode"
> >>>>>>>>> where it
> >>>>>>>>> refreshes the screen after each typed character, like it
> >>>>>>>>> does when ddb
> >>>>>>>>> is active.
> >>>>>>>>
> >>>>>>>> Looks like that's the case: for some reason the callouts
> >>>>>>>> don't work.
> >>>>>>>> This trivial hack is a (mostly) working workaround:
> >>>>>>>>
> >>>>>>>> Index: svn/head/sys/kern/kern_cons.c
> >>>>>>>> =
> >>>>>>>> ==
> >>>>>>>> --- svn/head/sys/kern/kern_cons.c (revision 297210)
> >>>>>>>> +++ svn/head/sys/kern/kern_cons.c (working copy)
> >>>>>>>> @@ -430,6 +430,7 @@ cngets(char *cp, size_t size, int
> >>>>>>>> visible)
> >>>>>>>>   lp = cp;
> >>>>>>>>   end = cp + size - 1;
> >>>>>>>>   for (;;) {
> >>>>>>>> + pause("meh", 1);
> >>>>>>>
> >>>>>>> Could you please explain how this works to me? Does calling
> >>>>>>> pause() here
> >>>>>>> give a chance to interrupt handlers or other threads of
> >>>>>>> running?
> >>>>>>
> >>>>>> It looks like it allows the callout to run.  I've did an
> >>>>>> experiment
> >>>>>> and added a simple callout that printed something each second;
> >>>>>> during
> >>>>>> the root mount prompt it doesn't get run unless you type '.',
> >>>>>> which
> >>>>>> calls pause(9).
> >>>>>
> >>>>> Kernel threads run with absolute priorities, so if somehow this
> >>>>> threads
> >>>>> happen to have higher or equal priority then callout thread, or the
> >>>>> kernel is built without PREEMPTION, then the last may never be
> >>>>> executed
> >>>>> until this thread get to sleep or at least call sched_yield().
> >>>>
> >>>> The callout's td_priority seems to be 40; the thread running the
> >>>> prompt
> >>>> is 84, so it's lower.
> >>>>
> >>>> I've just noticed another curious thing, though: when you press
> >>>> ScrLk,
> >>>> the screen gets immediately refreshed; also, pressing arrows works
> >>>> just
> >>>> the way it should.  In other words, the refresh is broken except for
> >>>> the ScrlLk mode, where it works as it should.
> >>>
> >>> Since cngets() is used only by the mountroot prompt and the geli pw
> >>> entry, pausing/yielding within the input loop seems like a good idea.
> >>>  It would allow for things like plugging in a usb device and having it
> >>> actually appear without having to enter a '.' several times.
> >>>
> >>> It would be nice if the pause were done with pause_sbt() and a shorter
> >>> timeout, maybe a millisecond or even 100uS.  Otherwise things like
> >>> pasting text at that prompt in a serial console is likely to drop
> >>> chars.
> >>>
> >>> Hmmm... speaking of the geli pw prompt... what's the locking situation
> >>> there?  Will there be any problems calling pause() from that context?
> >>
> >> PVM isn't an ideal

Re: svn commit: r297190 - head/sys/kern

[Edward Tomasz Napierała]

On 0324T1015, Warner Losh wrote:
> On Thu, Mar 24, 2016 at 9:46 AM, Ian Lepore <i...@freebsd.org> wrote:
> 
> > On Thu, 2016-03-24 at 16:01 +0100, Edward Tomasz Napierała wrote:
> > > On 0324T1609, Alexander Motin wrote:
> > > > On 24.03.16 15:42, Edward Tomasz Napierała wrote:
> > > > > On 0324T1032, Jean-Sébastien Pédron wrote:
> > > > > > On 23/03/2016 18:45, Edward Tomasz Napierala wrote:
> > > > > > > > So maybe callouts are disabled in this situation. If there
> > > > > > > > is a way to
> > > > > > > > detect that, then vt(4) can go back to a "synchronous mode"
> > > > > > > > where it
> > > > > > > > refreshes the screen after each typed character, like it
> > > > > > > > does when ddb
> > > > > > > > is active.
> > > > > > >
> > > > > > > Looks like that's the case: for some reason the callouts
> > > > > > > don't work.
> > > > > > > This trivial hack is a (mostly) working workaround:
> > > > > > >
> > > > > > > Index: svn/head/sys/kern/kern_cons.c
> > > > > > > =
> > > > > > > ==
> > > > > > > --- svn/head/sys/kern/kern_cons.c (revision 297210)
> > > > > > > +++ svn/head/sys/kern/kern_cons.c (working copy)
> > > > > > > @@ -430,6 +430,7 @@ cngets(char *cp, size_t size, int
> > > > > > > visible)
> > > > > > >   lp = cp;
> > > > > > >   end = cp + size - 1;
> > > > > > >   for (;;) {
> > > > > > > + pause("meh", 1);
> > > > > >
> > > > > > Could you please explain how this works to me? Does calling
> > > > > > pause() here
> > > > > > give a chance to interrupt handlers or other threads of
> > > > > > running?
> > > > >
> > > > > It looks like it allows the callout to run.  I've did an
> > > > > experiment
> > > > > and added a simple callout that printed something each second;
> > > > > during
> > > > > the root mount prompt it doesn't get run unless you type '.',
> > > > > which
> > > > > calls pause(9).
> > > >
> > > > Kernel threads run with absolute priorities, so if somehow this
> > > > threads
> > > > happen to have higher or equal priority then callout thread, or the
> > > > kernel is built without PREEMPTION, then the last may never be
> > > > executed
> > > > until this thread get to sleep or at least call sched_yield().
> > >
> > > The callout's td_priority seems to be 40; the thread running the
> > > prompt
> > > is 84, so it's lower.
> > >
> > > I've just noticed another curious thing, though: when you press
> > > ScrLk,
> > > the screen gets immediately refreshed; also, pressing arrows works
> > > just
> > > the way it should.  In other words, the refresh is broken except for
> > > the ScrlLk mode, where it works as it should.
> >
> > Since cngets() is used only by the mountroot prompt and the geli pw
> > entry, pausing/yielding within the input loop seems like a good idea.
> >  It would allow for things like plugging in a usb device and having it
> > actually appear without having to enter a '.' several times.
> >
> > It would be nice if the pause were done with pause_sbt() and a shorter
> > timeout, maybe a millisecond or even 100uS.  Otherwise things like
> > pasting text at that prompt in a serial console is likely to drop
> > chars.
> >
> > Hmmm... speaking of the geli pw prompt... what's the locking situation
> > there?  Will there be any problems calling pause() from that context?
> >
> 
> PVM isn't an ideal priority to wait at. PWAIT would be better. However,
> if the only reason to call pause is run the scheduler after each character,
> perhaps a better solution would be to call kern_yield() instead? We could
> do that instead of cpu_waitspin() inside of cngetc, but that would break
> the debugger's use of it

I think we should first try to figure out why this doesn't work in the first
place.

Basically, even though the interrupts are running, scheduler seems to be ok,
and the thread that's calling this has a lower priority than the callouts
thread, it can't be preempted.  This doesn't seem to be caused by vt(4);
with syscons the callouts don't get called either (it just doesn't break
the echo in this case).  To demonstrate the problem you can add add
a callout that calls printf each second and then does an infinite loop.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r297190 - head/sys/kern

[Edward Tomasz Napierała]

On 0324T1609, Alexander Motin wrote:
> On 24.03.16 15:42, Edward Tomasz Napierała wrote:
> > On 0324T1032, Jean-Sébastien Pédron wrote:
> >> On 23/03/2016 18:45, Edward Tomasz Napierala wrote:
> >>>> So maybe callouts are disabled in this situation. If there is a way to
> >>>> detect that, then vt(4) can go back to a "synchronous mode" where it
> >>>> refreshes the screen after each typed character, like it does when ddb
> >>>> is active.
> >>>
> >>> Looks like that's the case: for some reason the callouts don't work.
> >>> This trivial hack is a (mostly) working workaround:
> >>>
> >>> Index: svn/head/sys/kern/kern_cons.c
> >>> ===
> >>> --- svn/head/sys/kern/kern_cons.c (revision 297210)
> >>> +++ svn/head/sys/kern/kern_cons.c (working copy)
> >>> @@ -430,6 +430,7 @@ cngets(char *cp, size_t size, int visible)
> >>>   lp = cp;
> >>>   end = cp + size - 1;
> >>>   for (;;) {
> >>> + pause("meh", 1);
> >>
> >> Could you please explain how this works to me? Does calling pause() here
> >> give a chance to interrupt handlers or other threads of running?
> > 
> > It looks like it allows the callout to run.  I've did an experiment
> > and added a simple callout that printed something each second; during
> > the root mount prompt it doesn't get run unless you type '.', which
> > calls pause(9).
> 
> Kernel threads run with absolute priorities, so if somehow this threads
> happen to have higher or equal priority then callout thread, or the
> kernel is built without PREEMPTION, then the last may never be executed
> until this thread get to sleep or at least call sched_yield().

The callout's td_priority seems to be 40; the thread running the prompt
is 84, so it's lower.

I've just noticed another curious thing, though: when you press ScrLk,
the screen gets immediately refreshed; also, pressing arrows works just
the way it should.  In other words, the refresh is broken except for
the ScrlLk mode, where it works as it should.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r297190 - head/sys/kern

[Edward Tomasz Napierała]

On 0324T1032, Jean-Sébastien Pédron wrote:
> On 23/03/2016 18:45, Edward Tomasz Napierala wrote:
> >> So maybe callouts are disabled in this situation. If there is a way to
> >> detect that, then vt(4) can go back to a "synchronous mode" where it
> >> refreshes the screen after each typed character, like it does when ddb
> >> is active.
> > 
> > Looks like that's the case: for some reason the callouts don't work.
> > This trivial hack is a (mostly) working workaround:
> > 
> > Index: svn/head/sys/kern/kern_cons.c
> > ===
> > --- svn/head/sys/kern/kern_cons.c   (revision 297210)
> > +++ svn/head/sys/kern/kern_cons.c   (working copy)
> > @@ -430,6 +430,7 @@ cngets(char *cp, size_t size, int visible)
> > lp = cp;
> > end = cp + size - 1;
> > for (;;) {
> > +   pause("meh", 1);
> 
> Could you please explain how this works to me? Does calling pause() here
> give a chance to interrupt handlers or other threads of running?

It looks like it allows the callout to run.  I've did an experiment
and added a simple callout that printed something each second; during
the root mount prompt it doesn't get run unless you type '.', which
calls pause(9).

And, for the record, https://reviews.freebsd.org/D5724 doesn't fix
the problem.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r292740 - in head/sys: dev/cxgbe/cxgbei modules/cxgbe modules/cxgbe/cxgbei

[Edward Tomasz Napierała]

On 1226T0605, Navdeep Parhar wrote:
> Author: np
> Date: Sat Dec 26 06:05:21 2015
> New Revision: 292740
> URL: https://svnweb.freebsd.org/changeset/base/292740
> 
> Log:
>   cxgbei: Hardware accelerated iSCSI target and initiator for TOE capable
>   cards supported by cxgbe(4).
>   
>   On the host side this driver interfaces with the storage stack via the
>   ICL (iSCSI Common Layer) in the kernel.  On the wire the traffic is
>   standard iSCSI (SCSI over TCP as per RFC 3720/7143 etc.) that
>   interoperates with all other standards compliant implementations.  The
>   driver is layered on top of the TOE driver (t4_tom) and promotes
>   connections being handled by t4_tom to iSCSI ULP (Upper Layer Protocol)
>   mode.  Hardware assistance in this mode includes:
>   
>   - Full TCP processing.
>   - iSCSI PDU identification and recovery within the TCP stream.
>   - Header and/or data digest insertion (tx) and verification (rx).
>   - Zero copy (both tx and rx).
>   
>   Man page will follow in a separate commit in a couple of weeks.

Thank you!  :-)

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r291911 - head/sys/dev/iscsi

[Edward Tomasz Napierała]

On 1207T0256, Steven Hartland wrote:
> Author: smh
> Date: Mon Dec  7 02:56:08 2015
> New Revision: 291911
> URL: https://svnweb.freebsd.org/changeset/base/291911
> 
> Log:
>   Fix panic on shutdown due to iscsi event priority
>   
>   iscsi's shutdown_pre_sync prio was SHUTDOWN_PRI_FIRST which caused it to
>   run before other high priority handlers such as filesystems e.g. ZFS.
>   
>   This meant the iscsi sessions where removed before the ZFS geom consumer
>   was closed, resulting in a panic from g_access calls on debug kernels
>   due to negative acr.
>   
>   Instead use the same as the old iscsi_initiator SHUTDOWN_PRI_DEFAULT-1
>   which allows it to run before dashutdown etc but after filesystems.

I think this might be backwards.  Have you tested the following scenario:

1. Establish the iSCSI session
2. Mount the LUN somewhere
3. Make the target not reachable (eg ifconfig down), so that the
   initiator tries to reconnect
4. Try to shutdown?

Basically, the point of this code is to disable reconnects when there
won't be any iscsid(8) instance to handle them.  The iscsi_shutdown()
isn't supposed to remove working iSCSI sessions.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r291911 - head/sys/dev/iscsi

[Edward Tomasz Napierała]

On 1207T1134, Steven Hartland wrote:
> 
> 
> On 07/12/2015 11:22, Edward Tomasz Napierała wrote:
> > On 1207T0256, Steven Hartland wrote:
> >> Author: smh
> >> Date: Mon Dec  7 02:56:08 2015
> >> New Revision: 291911
> >> URL: https://svnweb.freebsd.org/changeset/base/291911
> >>
> >> Log:
> >>Fix panic on shutdown due to iscsi event priority
> >>
> >>iscsi's shutdown_pre_sync prio was SHUTDOWN_PRI_FIRST which caused it to
> >>run before other high priority handlers such as filesystems e.g. ZFS.
> >>
> >>This meant the iscsi sessions where removed before the ZFS geom consumer
> >>was closed, resulting in a panic from g_access calls on debug kernels
> >>due to negative acr.
> >>
> >>Instead use the same as the old iscsi_initiator SHUTDOWN_PRI_DEFAULT-1
> >>which allows it to run before dashutdown etc but after filesystems.
> > I think this might be backwards.  Have you tested the following scenario:
> >
> > 1. Establish the iSCSI session
> > 2. Mount the LUN somewhere
> > 3. Make the target not reachable (eg ifconfig down), so that the
> > initiator tries to reconnect
> > 4. Try to shutdown?
> >
> > Basically, the point of this code is to disable reconnects when there
> > won't be any iscsid(8) instance to handle them.  The iscsi_shutdown()
> > isn't supposed to remove working iSCSI sessions.
> I didn't notice your change in head / stable/10 compared to releng, so 
> I'm testing further updates now.

Well, the difference is pretty crucial here - previously, the purpose of
iscsi_shutdown() was to disconnect the sessions during shutdown.  Now it
doesn't do that at all - established connections are kept intact, it just
disables reconnections.  And for this to work, it needs to happen as soon
as possible during shutdown.

> Currently shutdown is run too early and actually doesn't actually 
> terminate any sessions.

What does "currently" mean in this context?

> My current theory is the issue you tried to fix in 286226 is actually 
> caused by the iscsi_maintenance_thread preferring reconnect over 
> terminate, so what you where seeing was the terminate never action for 
> reconnecting sessions. I'm currently trying to validate that theory now.
> 
> If I'm right then the fix should be to revert the majority of 286226 and 
> move the processing of terminate above reconnect in 
> iscsi_maintenance_thread.

I don't think that would work.  The reason that the sessions don't
terminate (in 11-CURRENT) is that they are not supposed to - unmounting
filesystems will write data to LUNs, and we don't want to break that.
The only reason iscsi_shutdown() exists is to prevent hang on shutdown
when there are sessions that require reconnection and there is no way
for it to succeed, because of iscsid is not running.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r287934 - head/sys/boot/efi/loader

[Edward Tomasz Napierała]

On 0917T1630, John Baldwin wrote:
> On Thursday, September 17, 2015 10:30:15 PM Bjoern A. Zeeb wrote:
> > 
> > > On 17 Sep 2015, at 20:43 , John Baldwin  wrote:
> > > 
> > > On Thursday, September 17, 2015 08:36:47 PM John Baldwin wrote:
> > >> Author: jhb
> > >> Date: Thu Sep 17 20:36:46 2015
> > >> New Revision: 287934
> > >> URL: https://svnweb.freebsd.org/changeset/base/287934
> > >> 
> > >> Log:
> > >>  The EFI boot loader allocates a single chunk of contiguous memory to
> > >>  hold the kernel, modules, and any other loaded data.  This memory block
> > >>  is relocated to the kernel's expected location during the transfer of
> > >>  control from the loader to the kernel.
> > >> 
> > >>  The GENERIC kernel on amd64 has recently grown such that a kernel + 
> > >> zfs.ko
> > >>  no longer fits in the default staging size.  Bump the default size from
> > >>  32MB to 48MB to provide more breathing room.
> > > 
> > > I believe that this should work fine for any system with 64MB of RAM.  One
> > > downside of the static size is that the loader fails if it can't allocate
> > > a contiguous staging size (it isn't able to grow the staging area on
> > > demand).
> > 
> > how do md_images work in that case?
> 
> The md_image has to fit into the same staging area (kernel plus any other
> files loaded by the loader including modules and md_images all have to fit
> in the staging area).  That was the original motivation for making the
> staging area a build-time tunable rather than always hardcoded at 32MB so
> that people who wished to deploy a large md_image can use a make flag to
> build a loader with a larger staging size (I tested this with a 200+MB
> mfsroot).

What would be required to get rid of that limitation altogether, ie make
it dynamic?  Right now it's quite a regression compared to the usual
(non-UEFI) loader.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r287405 - head/sys/geom

[Edward Tomasz Napierała]

On 0902T1729, Warner Losh wrote:
> Author: imp
> Date: Wed Sep  2 17:29:30 2015
> New Revision: 287405
> URL: https://svnweb.freebsd.org/changeset/base/287405
> 
> Log:
>   After the introduction of direct dispatch, the pacing code in g_down()
>   broke in two ways. One, the pacing variable was accessed in multiple
>   threads in an unsafe way. Two, since large numbers of I/O could come
>   down from the buf layer at one time, large numbers of allocation
>   failures could happen all at once, resulting in a huge pace value that
>   would limit I/Os to 10 IOPS for minutes (or even hours) at a
>   time. While a real solution to these problems requires substantial
>   work (to go to a no-allocation after the first model, or to have some
>   way to wait for more memory with some kind of reserve for pager and
>   swapper requests), it is relatively easy to make this simplistic
>   pacing less pathological.

Shouldn't we emit some warning the first time this happens, to aid
in debugging strange IO performance degradation?  Something like...

> @@ -688,7 +699,7 @@ g_io_deliver(struct bio *bp, int error)
>   bp->bio_driver2 = NULL;
>   bp->bio_pflags = 0;
>   g_io_request(bp, cp);

if (warned_about_pace == 0) {
printf("WARNING: GEOM io allocation failed; expect reduced IO 
performance\n");
warned_about_pace = 1;
}

> - pace++;
> + pace = 1;
>   return;
>  }

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r285384 - head/sys/kern

[Edward Tomasz Napierała]

On 0711T1121, Konstantin Belousov wrote:
 Author: kib
 Date: Sat Jul 11 11:21:56 2015
 New Revision: 285384
 URL: https://svnweb.freebsd.org/changeset/base/285384
 
 Log:
   Do not allow creation of the dirty buffers for the dead buffer
   objects, i.e. for buffer objects which vnode was reclaimed.  Buffer
   cache cannot write such buffers.  Return the error and discard the
   buffer immediately on write attempt.
   
   BO_DIRTY now always set during vnode reclamation, since it is used not
   only for the INVARIANTS checks.  Do allow placement of the clean
   buffers on dead bufobj list, otherwise filesystems cannot use bufcache
   at all after the devvp reclaim.

Note that it also fixes the dead bo panic that happens eg after
yanking a mounted flash drive.

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r280183 - in head/sys: dev/drm2 dev/drm2/i915 dev/drm2/radeon dev/drm2/ttm modules modules/drm2 modules/drm2/drm2 modules/drm2/radeonkms

[Edward Tomasz Napierała]

On 0317T2302, Jean-Sébastien Pédron wrote:
 On 17.03.2015 21:29, Edward Tomasz Napierała wrote:
o  Support for the setmaster/dropmaster ioctls. For instance, they
   are used to run multiple X servers simultaneously.
  
  Does it fix fast user switching in Xorg/GNOME, by any chance?
 
 Maybe, I don't know how this works in GNOME. But as long as it relies on
 two X sessions running at the same time, then yes it surely helps.

I've just tested, and user switching in GNOME works now.  Yay!

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r280183 - in head/sys: dev/drm2 dev/drm2/i915 dev/drm2/radeon dev/drm2/ttm modules modules/drm2 modules/drm2/drm2 modules/drm2/radeonkms

[Edward Tomasz Napierała]

On 0317T1850, Jean-Sebastien Pedron wrote:
 Author: dumbbell
 Date: Tue Mar 17 18:50:33 2015
 New Revision: 280183
 URL: https://svnweb.freebsd.org/changeset/base/280183
 
 Log:
   drm: Update the device-independent code to match Linux 3.8.13
   
   This update brings few features:
   o  Support for the setmaster/dropmaster ioctls. For instance, they
  are used to run multiple X servers simultaneously.

Does it fix fast user switching in Xorg/GNOME, by any chance?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r279603 - in head: bin/rcp usr.bin/rlogin usr.bin/rsh

[Edward Tomasz Napierała]

On 0305T1555, Gleb Smirnoff wrote:
 On Thu, Mar 05, 2015 at 03:40:37PM +0300, Slawa Olhovchenkov wrote:
 S  nc(1), which is a pure socket testing tool. For telnet(1) this
 S  capability is a side effect.
 S 
 S You don't try this in practice.
 S % nc zxy.spb.ru 81
 S %
 S % telnet zxy.spb.ru 81
 S Trying 195.70.199.98...
 S telnet: connect to address 195.70.199.98: Connection refused
 S telnet: Unable to connect to remote host
 S 
 S nc is not usable. 
 
 Yes, this how unix way works. Command has return value, or
 you can use '-v', of you want it to be chatty.

Actually, not giving error message on error by default seems quite
broken to me.  Standard UNIX utilities don't fail silently:

% cat /bin/nope
cat: /bin/nope: No such file or directory

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r278034 - head/sys/dev/ahci

[Edward Tomasz Napierała]

On 0201T2000, Steven Hartland wrote:
 Author: smh
 Date: Sun Feb  1 20:00:08 2015
 New Revision: 278034
 URL: https://svnweb.freebsd.org/changeset/base/278034
 
 Log:
   Add a quirk to limit AHCI MSI vectors to one
   
   In 10.1-RELEASE the default number of MSI vectors used was changed from one
   to as many vectors as the HW supports.
   
   This change resulted in an ahci timeouts regression when running on AMD
   SB7x0/SB8x0/SB9x0 hardware, so its now limited to 1 MSI by default using
   this new quirk.

Could it fix https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=195349?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r274962 - head/sys/cam/ctl

[Edward Tomasz Napierała]

On 1124T1137, Alexander Motin wrote:
 Author: mav
 Date: Mon Nov 24 11:37:27 2014
 New Revision: 274962
 URL: https://svnweb.freebsd.org/changeset/base/274962
 
 Log:
   Replace home-grown CTL IO allocator with UMA.
   
   Old allocator created significant lock congestion protecting its lists
   of preallocated I/Os, while UMA provides much better SMP scalability.
   The downside of UMA is lack of reliable preallocation, that could guarantee
   successful allocation in non-sleepable environments.  But careful code
   review shown, that only CAM target frontend really has that requirement.
   Fix that making that frontend preallocate and statically bind CTL I/O for
   every ATIO/INOT it preallocates any way.  That allows to avoid allocations
   in hot I/O path.  Other frontends either may sleep in allocation context
   or can properly handle allocation errors.
   
   On 40-core server with 6 ZVOL-backed LUNs and 7 iSCSI client connections
   this change increases peak performance from ~700K to 1M IOPS!  Yay! :)

Hell yeah!

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r273806 - in head/contrib/ofed: libcxgb4 libcxgb4/src usr.lib usr.lib/libcxgb4

[Edward Tomasz Napierała]

On 1029T1123, Navdeep Parhar wrote:
 On Wed, Oct 29, 2014 at 10:56:04AM +0100, Edward Tomasz Napierała wrote:
  On 1029T0115, Navdeep Parhar wrote:
   Author: np
   Date: Wed Oct 29 01:15:48 2014
   New Revision: 273806
   URL: https://svnweb.freebsd.org/changeset/base/273806
   
   Log:
 Userspace library for Chelsio's Terminator 5 based iWARP RNICs (pretty
 much every T5 card that does _not_ have -SO in its name is RDMA
 capable).
  
  Yay!  This means we could add iSER without using the ICL_PROXY hack.
  Well, assuming it's possible to hand off RDMA connection from userspace
  to the kernel.  Is it?
 
 Yes, this should be doable.  The connection is just another TCP endpoint
 tracked like all others in the kernel.

:-)

 By the way, iSER is an unnecessary layer if you're using a T5 NIC.
 It'll work, sure, but you'll run iSER/RDMA/TOE when you could simply run
 iSCSI/TOE with full zero copy everywhere.  Comes out to the same result
 with a much simpler stack.  I think iSER makes sense for gear that does
 RDMA but not iSCSI natively.

True.  IMHO the biggest difference is that iWARP is universal, ie.
the NIC doesn't need to have any iSCSI-specific functionality.  On
the other hand, iSER cannot talk to non-iSER, while iSCSI offload
in your NICs talks ordinary iSCSI on the wire.

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r273806 - in head/contrib/ofed: libcxgb4 libcxgb4/src usr.lib usr.lib/libcxgb4

[Edward Tomasz Napierała]

On 1029T0115, Navdeep Parhar wrote:
 Author: np
 Date: Wed Oct 29 01:15:48 2014
 New Revision: 273806
 URL: https://svnweb.freebsd.org/changeset/base/273806
 
 Log:
   Userspace library for Chelsio's Terminator 5 based iWARP RNICs (pretty
   much every T5 card that does _not_ have -SO in its name is RDMA
   capable).

Yay!  This means we could add iSER without using the ICL_PROXY hack.
Well, assuming it's possible to hand off RDMA connection from userspace
to the kernel.  Is it?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r273549 - head/sys/kern

[Edward Tomasz Napierała]

Dnia 23 paź 2014 o godz. 20:38 John Baldwin j...@freebsd.org napisał(a):

 On Thursday, October 23, 2014 11:35:47 am Mateusz Guzik wrote:
 Author: mjg
 Date: Thu Oct 23 15:35:47 2014
 New Revision: 273549
 URL: https://svnweb.freebsd.org/changeset/base/273549
 
 Log:
  Avoid taking the lock in selfdfree when not needed.
 
 Modified:
  head/sys/kern/sys_generic.c
 
 Modified: head/sys/kern/sys_generic.c
 ==
 --- head/sys/kern/sys_generic.cThu Oct 23 15:16:40 2014(r273548)
 +++ head/sys/kern/sys_generic.cThu Oct 23 15:35:47 2014(r273549)
 @@ -1600,10 +1600,11 @@ static void
 selfdfree(struct seltd *stp, struct selfd *sfp)
 {
STAILQ_REMOVE(stp-st_selq, sfp, selfd, sf_link);
 -mtx_lock(sfp-sf_mtx);
 -if (sfp-sf_si)
 +if (sfp-sf_si != NULL) {
 +mtx_lock(sfp-sf_mtx);
TAILQ_REMOVE(sfp-sf_si-si_tdlist, sfp, sf_threads);
 -mtx_unlock(sfp-sf_mtx);
 +mtx_unlock(sfp-sf_mtx);
 +}
uma_zfree(selfd_zone, sfp);
 
 How do you ensure that the value you read for sf_si here is up to date?  In 
 particular, if a thread is selecting on multiple fds and one awakens it,
 another fd can invoke selwakeup() while the thread is in seltdclear().
 In that case, you might see a stale value of sf_si and not realize it is 
 cleared by the selwakeup() after you get the lock and you will invoke
 TAILQ_REMOVE an extra time.

FWIW, I've just hit a panic in selfdfree().

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r265498 - stable/10/sys/dev/iscsi

[Edward Tomasz Napierała]

Wiadomość napisana przez Robert Watson w dniu 7 maj 2014, o godz. 13:49:
 On Wed, 7 May 2014, Edward Tomasz Napierala wrote:
 
 Author: trasz
 Date: Wed May  7 06:38:19 2014
 New Revision: 265498
 URL: http://svnweb.freebsd.org/changeset/base/265498
 
 Log:
 MFC r264025:
 
 Get rid of the autoscaling, instead just set socket buffer sizes
 in the usual way.  The only thing the old code did was making things
 less predictable.
 
 Does this mean that the autoscaling algorithm needs refining?  The problem 
 with disabling autoscaling is that the code may, in the future, fail to 
 benefit from further global refinements, as old code that hard-coded sizes 
 now fails to do -- and that if we don't refine the autoscaling model, other 
 applications may fail to benefit.

The commit message here was misleading - what got replaced was
setting the socket buffer size by hand to a value  computed from
negotiated maximum PDU size.  Now it's set to constant size.

The reason we do not use autoscaling here is not performance.
It's because iSCSI code can't transmit a part of PDU; it always waits
until there is room for the whole thing (PDUs can be up to about 128kB
in size). If the socket buffer size somehow got autoscaled below
that amount, the transmit would stop indefinitely.  There is a somewhat
similar situation on the receive side.

So, the right solution would be to either use autoscaling, but with
a guarantee that the socket buffer won't shrink below some specified
value, or have a way to determine that the socket buffer won't get any
bigger anytime soon and so the iSCSI should transmit or receive a part
of PDU.  Current setting by hand is just a workaround, albeit pretty
well working one.

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r264025 - head/sys/dev/iscsi

[Edward Tomasz Napierała]

Wiadomość napisana przez Ian Lepore w dniu 2 kwi 2014, o godz. 15:24:

 On Tue, 2014-04-01 at 22:03 +, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Tue Apr  1 22:03:03 2014
 New Revision: 264025
 URL: http://svnweb.freebsd.org/changeset/base/264025
 
 Log:
  Get rid of the autoscaling, instead just set socket buffer sizes
  in the usual way.  The only thing the old code did was making things
  less predictable.
 
  Sponsored by:   The FreeBSD Foundation
 
 Modified:
  head/sys/dev/iscsi/icl.c
 
 Modified: head/sys/dev/iscsi/icl.c
 ==
 --- head/sys/dev/iscsi/icl.c Tue Apr  1 21:54:20 2014(r264024)
 +++ head/sys/dev/iscsi/icl.c Tue Apr  1 22:03:03 2014(r264025)
 @@ -68,6 +68,14 @@ TUNABLE_INT(kern.icl.partial_receive_le
 SYSCTL_INT(_kern_icl, OID_AUTO, partial_receive_len, CTLFLAG_RW,
 partial_receive_len, 1 * 1024, Minimum read size for partially 
 received 
 data segment);
 +static int sendspace = 1048576;
 +TUNABLE_INT(kern.icl.sendspace, sendspace);
 +SYSCTL_INT(_kern_icl, OID_AUTO, sendspace, CTLFLAG_RW,
 +sendspace, 1048576, Default send socket buffer size);
 +static int recvspace = 1048576;
 +TUNABLE_INT(kern.icl.recvspace, recvspace);
 +SYSCTL_INT(_kern_icl, OID_AUTO, recvspace, CTLFLAG_RW,
 +recvspace, 1048576, Default receive socket buffer size);
 
 static uma_zone_t icl_conn_zone;
 static uma_zone_t icl_pdu_zone;
 @@ -1008,7 +1016,7 @@ icl_conn_free(struct icl_conn *ic)
 static int
 icl_conn_start(struct icl_conn *ic)
 {
 -size_t bufsize;
 +size_t minspace;
  struct sockopt opt;
  int error, one = 1;
 
 @@ -1029,18 +1037,28 @@ icl_conn_start(struct icl_conn *ic)
  ICL_CONN_UNLOCK(ic);
 
  /*
 - * Use max available sockbuf size for sending.  Do it manually
 - * instead of sbreserve(9) to work around resource limits.
 + * For sendspace, this is required because the current code cannot
 + * send a PDU in pieces; thus, the minimum buffer size is equal
 + * to the maximum PDU size.  +4 is to account for possible padding.
   *
 - * XXX: This kind of sucks.  On one hand, we don't currently support
 - *  sending a part of data segment; we always do it in one piece,
 - *  so we have to make sure it can fit in the socket buffer.
 - *  Once I've implemented partial send, we'll get rid of this
 - *  and use autoscaling.
 + * What we should actually do here is to use autoscaling, but set
 + * some minimal buffer size to minspace.  I don't know a way to do
 + * that, though.
   */
 -bufsize = (sizeof(struct iscsi_bhs) +
 -ic-ic_max_data_segment_length) * 8;
 -error = soreserve(ic-ic_socket, bufsize, bufsize);
 +minspace = sizeof(struct iscsi_bhs) + ic-ic_max_data_segment_length +
 +ISCSI_HEADER_DIGEST_SIZE + ISCSI_DATA_DIGEST_SIZE + 4;
 +if (sendspace  minspace) {
 +ICL_WARN(kern.icl.sendspace too low; must be at least %jd,
 +minspace);
 +sendspace = minspace;
 +}
 +if (recvspace  minspace) {
 +ICL_WARN(kern.icl.recvspace too low; must be at least %jd,
 +minspace);
 +recvspace = minspace;
 +}
 +
 
 The %jd on these is causing build failures on 32-bit arches due to
 size_t being only 32 bits.

Right.  Should be fixed now, sorry for breakage.

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r263978 - head/sys/cam/ctl

[Edward Tomasz Napierała]

Wiadomość napisana przez Kubilay Kocak w dniu 1 kwi 2014, o godz. 10:08:
 On 1/04/2014 7:49 AM, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Mon Mar 31 20:49:33 2014
 New Revision: 263978
 URL: http://svnweb.freebsd.org/changeset/base/263978
 
 Log:
  Make it possible to have multiple CTL worker threads.  Leave the default
  of 1 for now.
 
  Sponsored by:   The FreeBSD Foundation
 
 Modified:
  head/sys/cam/ctl/ctl.c
 
 Modified: head/sys/cam/ctl/ctl.c
 ==
 --- head/sys/cam/ctl/ctl.c   Mon Mar 31 19:58:08 2014(r263977)
 +++ head/sys/cam/ctl/ctl.c   Mon Mar 31 20:49:33 2014(r263978)
 @@ -60,6 +60,7 @@ __FBSDID($FreeBSD$);
 #include sys/ioccom.h
 #include sys/queue.h
 #include sys/sbuf.h
 +#include sys/smp.h
 #include sys/endian.h
 #include sys/sysctl.h
 
 @@ -320,6 +321,10 @@ static int ctl_is_single = 1;
 static int index_to_aps_page;
 
 SYSCTL_NODE(_kern_cam, OID_AUTO, ctl, CTLFLAG_RD, 0, CAM Target Layer);
 +static int worker_threads = 1;
 +TUNABLE_INT(kern.cam.ctl.worker_threads, worker_threads);
 +SYSCTL_INT(_kern_cam_ctl, OID_AUTO, worker_threads, CTLFLAG_RDTUN,
 +worker_threads, 1, Number of worker threads);
 
 /*
  * Serial number (0x80), device id (0x83), and supported pages (0x00)
 @@ -950,10 +955,7 @@ ctl_init(void)
  struct ctl_frontend *fe;
  struct ctl_lun *lun;
 uint8_t sc_id =0;
 -#if 0
 -int i;
 -#endif
 -int error, retval;
 +int i, error, retval;
  //int isc_retval;
 
  retval = 0;
 @@ -1085,17 +1087,35 @@ ctl_init(void)
  mtx_unlock(softc-ctl_lock);
 #endif
 
 -error = kproc_create(ctl_work_thread, softc, softc-work_thread, 0, 0,
 - ctl_thrd);
 -if (error != 0) {
 -printf(error creating CTL work thread!\n);
 -mtx_lock(softc-ctl_lock);
 -ctl_free_lun(lun);
 -mtx_unlock(softc-ctl_lock);
 -ctl_pool_free(internal_pool);
 -ctl_pool_free(emergency_pool);
 -ctl_pool_free(other_pool);
 -return (error);
 +if (worker_threads  MAXCPU || worker_threads == 0) {
 +printf(invalid kern.cam.ctl.worker_threads value; 
 +setting to 1);
 +worker_threads = 1;
 +} else if (worker_threads  0) {
 
 Why is it that the  0 case is special enough that it gets the mp_ncpus
 check below, but the == 0 and  MAXCPU cases don't?

It's to be able to set it to -1 (which will probably become the new default
soon) and let the code figure out the right number by itself.

 +if (mp_ncpus  2) {
 +/*
 + * Using more than two worker threads actually hurts
 + * performance due to lock contention.
 + */
 
 +worker_threads = 2;
 +} else {
 +worker_threads = 1;
 +}
 
 Are printf(Setting to N)'s worthwhile here as well given
 worker_threads is set to a value that wasn't specified by the user, as
 above?

The warning is to inform the user why the value supplied was ignored
as invalid.  The -1 setting is always valid.

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r261266 - in head: sys/dev/drm sys/kern sys/sys usr.sbin/jail

[Edward Tomasz Napierała]

Wiadomość napisana przez James Gritton w dniu 4 lut 2014, o godz. 14:49:
 On 2/4/2014 6:23 AM, Julian Elischer wrote:
 On 2/4/14, 3:40 PM, Robert N. M. Watson wrote:
 On 3 Feb 2014, at 23:53, Doug Ambrisko ambri...@ambrisko.com wrote:
 
 It's unfortunate that vimage requires jail.  I want to use vimage but
 not have the security restrictions of a jail.  To do this I patched
 jail to basically let everything through.  It would be nice to be
 able to run jail in an insecure mode which I understand is a contradition.
 I do use the jail infrastructure to set the uname*/getosreldate so
 that a specific jail thinks it is FreeBSD version blah.  Then I can ssh
 into that jail and pkg_add things, make ports etc.  I use this on
 my laptop running current on the base.  My other jails run various
 versions of FreeBSD.  I don't care about security in this case.
 
 vimage was not originally tied to jails. I can't remember why we decided to 
 do that :-)
 
 Leaving the smiley aside for the present, I remember that one - and
 it's closely tied to this discussion.  It was part of this more
 flexible vision of jails that had added features, of which security
 was just one (optional) part.  I thought of them as a more general
 encapsulation framework as needs would arise.

Just for the record, that's the exact same reason I didn't invent yet another
encapsulation mechanism for RCTL - the idea was to use jails when you need
any kind of nested hierarchy.

-- 
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r258909 - in head: sbin/mdconfig sys/dev/md sys/sys

[Edward Tomasz Napierała]

Wiadomość napisana przez Mateusz Guzik w dniu 4 gru 2013, o godz. 08:54:
 On Wed, Dec 04, 2013 at 07:38:24AM +, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Wed Dec  4 07:38:23 2013
 New Revision: 258909
 URL: http://svnweb.freebsd.org/changeset/base/258909
 
 Log:
  Add null backend to mdconfig(8).  This does exactly what the name
  suggests, and is somewhat useful for benchmarking.
 
 
 There is already a 'geom zero' module which I believe provides
 equivalent functionality.

Oh, the joys of software without manual pages.  Still, I'm leaning toward
leaving this in, in case someone needs more than one instance, or a particular
size.

-- 
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r258041 - head/lib/libc/posix1e

[Edward Tomasz Napierała]

Wiadomość napisana przez Robert Watson w dniu 14 lis 2013, o godz. 09:07:
 On Tue, 12 Nov 2013, Edward Tomasz Napierala wrote:
 
 Mention acl_get_brand_np(3).
 
 MFC after:   2 weeks
 Sponsored by:The FreeBSD Foundation
 
 Doing some writing recently, I did wonder if acl(3) and some of the other ACL 
 API man pages might need a bit more updating to be clear about support for 
 NFSv4 ACLs.  The user-facing man pages (getfacl(1), setfacl(1)) appear fully 
 updated, but not much mention of NFSv4 in the programmer documentation.

Hm, it should be already documented - from updated list of constants
in acl_add_perm(3) or acl_set_tag_type(3), to new functions, such as
acl_add_flag_np(3) or acl_set_entry_type_np(3).

-- 
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r255323 - in head/sys: amd64/conf i386/conf

[Edward Tomasz Napierała]

Wiadomość napisana przez Adrian Chadd adr...@freebsd.org w dniu 7 wrz 2013, o 
godz. 19:47:
 I'll be happy if someone does this right now, by populating a 
 /boot/loader.modules or something, and then force the fixing of loader to 
 cache metadata to make the reads faster.

I have no idea on what's the loader(8) state right now, but long time ago I''ve 
made a patch
that made it significantly faster by making caching actually work.  No idea if 
anyone picked
up the patch (http://people.freebsd.org/~trasz/fast-loader-3.diff), though.

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r249410 - in head/sys: amd64/conf arm/conf cam/ctl conf i386/conf ia64/conf modules/ctl sparc64/conf

[Edward Tomasz Napierała]

Wiadomość napisana przez Marius Strobl mar...@alchemy.franken.de w dniu 29 
lip 2013, o godz. 22:38:
 On Fri, Apr 12, 2013 at 04:25:03PM +, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Fri Apr 12 16:25:03 2013
 New Revision: 249410
 URL: http://svnweb.freebsd.org/changeset/base/249410
 
 Log:
  Remove ctl(4) from GENERIC.  Also remove 'options CTL_DISABLE'
  and kern.cam.ctl.disable tunable; those were introduced as a workaround
  to make it possible to boot GENERIC on low memory machines.
 
  With ctl(4) being built as a module and automatically loaded by ctladm(8),
  this makes CTL work out of the box.
 
 
 Uhm, shouldn't r249328 and the above change be MFCed to stable/9 in
 order to reduce the default memory footprint there?

Yup, my bad.  I'm waiting for reply from re@ whether it's a good thing
to do at this point of the release process.

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r249410 - in head/sys: amd64/conf arm/conf cam/ctl conf i386/conf ia64/conf modules/ctl sparc64/conf

[Edward Tomasz Napierała]

Wiadomość napisana przez Marius Strobl mar...@alchemy.franken.de w dniu 30 
lip 2013, o godz. 22:28:
 On Tue, Jul 30, 2013 at 09:57:21PM +0200, Edward Tomasz Napiera?a wrote:
 Wiadomo?? napisana przez Marius Strobl mar...@alchemy.franken.de w dniu 29 
 lip 2013, o godz. 22:38:
 On Fri, Apr 12, 2013 at 04:25:03PM +, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Fri Apr 12 16:25:03 2013
 New Revision: 249410
 URL: http://svnweb.freebsd.org/changeset/base/249410
 
 Log:
 Remove ctl(4) from GENERIC.  Also remove 'options CTL_DISABLE'
 and kern.cam.ctl.disable tunable; those were introduced as a workaround
 to make it possible to boot GENERIC on low memory machines.
 
 With ctl(4) being built as a module and automatically loaded by ctladm(8),
 this makes CTL work out of the box.
 
 
 Uhm, shouldn't r249328 and the above change be MFCed to stable/9 in
 order to reduce the default memory footprint there?
 
 Yup, my bad.  I'm waiting for reply from re@ whether it's a good thing
 to do at this point of the release process.
 
 
 Yeah, I saw that request and was looking into what it would actually
 take to remove ctl(4) from GENERICs in stable/9. Unfortunately, it's
 not exactly as straight forward as MFCing the two above revisions 1:1.
 For one, IMO kern.cam.ctl.disable shouldn't be removed there in case
 someone uses an old kernel configuration file having device ctl but
 relies on that tunable to work. That's no big deal, though, and makes
 the merge even less intrusive. However, the real problem is that ctl.ko
 currently is globally disabled in stable/9 as it doesn't build with PAE/
 XEN. Actually, the compiler is right to complain as a 32-bit void-pointer
 is casted to a 64-bit integer in that case. What I don't get so far is
 why on earth this doesn't break compilation in head ...

Damn.  Now that you mention it... yeah, it was me who disconnected it,
in r249530.  I had no idea what's going on either.

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r249939 - head/sys/cam/scsi

[Edward Tomasz Napierała]

Wiadomość napisana przez Steven Hartland w dniu 30 kwi 2013, o godz. 05:16:

[..]

 If anyone's interested the trace was:-
 #0  doadump (textdump=0) at pcpu.h:231
 #1  0x802f6d6e in db_dump (dummy=value optimized out, dummy2=0, 
 dummy3=0, dummy4=0x0) at 
 /usr/home/smh/freebsd/base/head/sys/ddb/db_command.c:543
 #2  0x802f683d in db_command (cmd_table=value optimized out) at 
 /usr/home/smh/freebsd/base/head/sys/ddb/db_command.c:449
 #3  0x802f65b4 in db_command_loop () at 
 /usr/home/smh/freebsd/base/head/sys/ddb/db_command.c:502
 #4  0x802f8f50 in db_trap (type=value optimized out, code=0) at 
 /usr/home/smh/freebsd/base/head/sys/ddb/db_main.c:231
 #5  0x805c0df3 in kdb_trap (type=9, code=0, tf=value optimized out) 
 at /usr/home/smh/freebsd/base/head/sys/kern/subr_kdb.c:654
 #6  0x8075580a in trap_fatal (frame=0xff823b8dc790, eva=value 
 optimized out) at /usr/home/smh/freebsd/base/head/sys/amd64/amd64/trap.c:867
 #7  0x807554b7 in trap (frame=value optimized out) at 
 /usr/home/smh/freebsd/base/head/sys/amd64/amd64/trap.c:224
 #8  0x8073e1f2 in calltrap () at 
 /usr/home/smh/freebsd/base/head/sys/amd64/amd64/exception.S:228
 #9  0x8029c860 in cam_periph_alloc (periph_ctor=0x802af410 
 proberegister, periph_oninvalidate=0xfe0019cfa200, periph_dtor=value 
 optimized out, periph_start=0xfe0015980a90, name=value optimized out, 
 type=2159638184,
   path=0xfe0015ad79a0, ac_callback=value optimized out, code=value 
 optimized out) at /usr/home/smh/freebsd/base/head/sys/cam/cam_periph.c:227
 #10 0x802aed6b in scsi_scan_lun (request_ccb=value optimized out) 
 at /usr/home/smh/freebsd/base/head/sys/cam/scsi/scsi_xpt.c:2266
 #11 0x802b2859 in scsi_scan_bus (periph=0x0, 
 request_ccb=0xfe00234df000) at 
 /usr/home/smh/freebsd/base/head/sys/cam/scsi/scsi_xpt.c:1969
 #12 0x802a66c5 in xpt_scanner_thread (dummy=value optimized out) at 
 /usr/home/smh/freebsd/base/head/sys/cam/cam_xpt.c:2411

I've seen a panic which is slightly different, but perhaps could be related:

#11 0x8085fde2 in __mtx_assert (c=0xff81aeccd8c0, 
what=value optimized out, 
file=0x80e6b0aa /home/trasz/p4/iscsi/sys/kern/kern_cons.c, 
line=128) at /home/trasz/p4/iscsi/sys/kern/kern_mutex.c:788
#12 0x802cc8e1 in xpt_compile_path (new_path=0xfe00150d7180, 
perph=0xfe00112e1e00, path_id=value optimized out, target_id=0, 
lun_id=0) at /home/trasz/p4/iscsi/sys/cam/cam_xpt.c:4664
#13 0x802cba1b in xpt_create_path (new_path_ptr=0xff81aeccdb50, 
perph=0xfe00112e1e00, path_id=2, target_id=0, lun_id=0)
at /home/trasz/p4/iscsi/sys/cam/cam_xpt.c:3398
#14 0x802dcca3 in scsi_scan_bus (periph=value optimized out, 
request_ccb=0xfe00157e4800)
at /home/trasz/p4/iscsi/sys/cam/scsi/scsi_xpt.c:1955
#15 0x802d0ca4 in xpt_scanner_thread (dummy=value optimized out)
at /home/trasz/p4/iscsi/sys/cam/cam_xpt.c:841

It looks like it's impossible for the lock to be not owned in that code
path, so it suggests some kind of memory corruption.  It can be easily
reproduced by creating several SIMs at the exact same time.

[..]

-- 
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r243246 - head/sbin/growfs

[Edward Tomasz Napierała]

Wiadomość napisana przez Gavin Atkinson w dniu 18 lis 2012, o godz. 20:25:
 On Sun, 18 Nov 2012, Edward Tomasz Napierala wrote:
 
 Author: trasz
 Date: Sun Nov 18 19:01:00 2012
 New Revision: 243246
 URL: http://svnweb.freebsd.org/changeset/base/243246
 
 Log:
  Make it possible to resize filesystems mounted read-write, using newly
  introduced UFS write suspension mechanism.
 
 Modified: head/sbin/growfs/growfs.c
 ==
 --- head/sbin/growfs/growfs.cSun Nov 18 18:57:19 2012
 (r243245)
 +++ head/sbin/growfs/growfs.cSun Nov 18 19:01:00 2012
 (r243246)
 @@ -1555,9 +1557,18 @@ main(int argc, char **argv)
  if (Nflag) {
  fso = -1;
  } else {
 -fso = open(device, O_WRONLY);
 -if (fso  0)
 -err(1, %s, device);
 +if (statfsp != NULL  (statfsp-f_flags  MNT_RDONLY) == 0) {
 +fso = open(_PATH_UFSSUSPEND, O_RDWR);
 +if (fso == -1)
 +err(1, unable to open %s, _PATH_UFSSUSPEND);
 +error = ioctl(fso, UFSSUSPEND, statfsp-f_fsid);
 +if (error != 0)
 +err(1, UFSSUSPEND);
 +} else {
 +fso = open(device, O_WRONLY);
 +if (fso  0)
 +err(1, %s, device);
 +}
  }
 
 It is excellent to see this functionality, and it will be very much 
 appreciated especially for people using virtual machines.

Thanks!

 All the way through later code there are calls to err() on failure.  
 What happens to the suspended filesystem if that happens and growfs exits 
 before the matching UFSRESUME?

Kernel releases the suspension when the process keeping the filedescriptor
exits.  It could be said that the UFSRESUME ioctl is there mostly
for completeness.  And the changes performed by growfs(8) are ordered in
a way that keeps the filesystem consistent, so it should be safe to ^C
growfs execution without breaking things.

-- 
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r238213 - head/sys/geom

[Edward Tomasz Napierała]

Wiadomość napisana przez Pawel Jakub Dawidek w dniu 8 lip 2012, o godz. 23:38:
 On Sun, Jul 08, 2012 at 12:53:37AM +0200, Edward Tomasz Napierała wrote:
 Wiadomość napisana przez Pawel Jakub Dawidek w dniu 7 lip 2012, o godz. 
 23:54:
 You will also notice that one of those fields were left for more
 universal method to handle various provider's property changes (ie.
 provider's name, apart from its mediasize). The initial patch was even
 published a year ago:
 
 http://people.freebsd.org/~pjd/patches/geom_property_change.patch
 
 Even if it was somehow totally not reusable it would at least give you a
 hint that mediasize is not the only thing that can change and if we are
 making that change it should be done right.
 
 I was not aware of that patch. [...]
 
 I'm afraid that's a lie. From IRC logs:
 
 pjd rwatson: 
 http://people.freebsd.org/~pjd/patches/geom_property_change.patch
 pjd rwatson: Not tested.
 trasz pjd: shouldn't there also be a flag for geom to veto resizing?
 trasz pjd: for classes that can't handle their consumers changing size?
 [the discussion was pretty long]

And when exactly was that?  A year ago?

 [...] What I've considered was to use attributes
 instead, but that would complicate notifying consumers about resizing
 and would require some special-casing in the attribute code.
 
 What attributes? The ones handled by BIO_GETATTR? They are about
 something totally different.

Yes, but they could be used to notify about mediasize change.

Now, I'm not sure if I like your approach.  You're trying to generalize
from a single case.  And even for that single case your approach would
require a special case to retaste the provider after updating mediasize,
and perhaps do the orphanisation stuff before.

Also, why would we want this generalisation?  Resize handling is completely
different from e.g. rename handling; why should we have a single method
to do both?  It's not that geom structures are like mbufs or vnodes, where
every byte counts.

 +  G_VALID_PROVIDER(pp);
 
 Is this your protection from a provider going away?
 
 Can you suggest a way to do it in a safe way that doesn't involve
 rewriting most of GEOM?
 
 I can only suggest not to rewrite GEOM because you didn't take the time
 to understand it.

Let me quote a comment from the top of geom_event.c:

/*
 * XXX: How do we in general know that objects referenced in events
 * have not been destroyed before we get around to handle the event ?
 */

So, can you suggest a way to do it in a safe way that doesn't involve
rewriting most of GEOM?

 Why is this safe to call the orphan method directly and not use
 g_orphan_provider()? I don't know if using g_orphan_provider() is safe
 to use here either, but I'm under impression that you assume no orphan
 method will ever drop the topology lock? We have tools to assert that,
 no need to introduce such weak assumptions.
 
 It's not that using g_orphan_provider() would be safer here - it simply
 wouldn't work.  The way it works is by adding providers to a queue
 (g_doorstep).  _Providers_, and we need to orphan individual consumers.
 So, this would involve rewriting the orphanisation mechanism.  Also,
 most of the classes were fixed by mav@ to handle this correctly, IIRC.
 
 By introducing such hacks you make the code unpredictable. The way
 g_orphan_provider() works is more than just calling geom's orphan
 method. Also, until now, when orphan method was called it meant that
 provider is going away, which is not true anymore. I'd like to believe
 that you carefully analysed what you changed here is safe, but based on
 your understanding of GEOM, I doubt that.

Look, I really appreciate you're looking at this just six months after
explicitly refusing to talk to me about the design, but it would be great
if it was a _technical_ discussion.

Now, the only reason for the orphaning during resizing is backward
compatibility with classes that don't know anything about the resize()
method, to make sure the provider doesn't get shrunk without them knowing. 
Your patch didn't do that, and perhaps we could just get rid of it.
I think the current approach is safer, though.

-- 
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r238213 - head/sys/geom

[Edward Tomasz Napierała]

Wiadomość napisana przez Pawel Jakub Dawidek w dniu 7 lip 2012, o godz. 23:54:
 On Sat, Jul 07, 2012 at 08:13:41PM +, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Sat Jul  7 20:13:40 2012
 New Revision: 238213
 URL: http://svn.freebsd.org/changeset/base/238213
 
 Log:
  Add a new GEOM method, resize(), which is called after provider size 
 changes.
  Add a new routine, g_resize_provider(), to use to notify GEOM about provider
  change.
 
  Reviewed by:mav
  Sponsored by:   FreeBSD Foundation
 [...]
 -void*spare2;
 +g_resize_t  *resize;
 [...]
 -void*spare1;
 +g_resize_t  *resize;
 [...]
 
 If you take the time to actually read the commit log from the change
 that added those spare fields, you will notice they were not added for
 you to consume them.

Perhaps it wasn't your original intent, but they are spares.  One of these
was already reused for some other task, btw.

 You will also notice that one of those fields were left for more
 universal method to handle various provider's property changes (ie.
 provider's name, apart from its mediasize). The initial patch was even
 published a year ago:
 
   http://people.freebsd.org/~pjd/patches/geom_property_change.patch
 
 Even if it was somehow totally not reusable it would at least give you a
 hint that mediasize is not the only thing that can change and if we are
 making that change it should be done right.

I was not aware of that patch.  What I've considered was to use attributes
instead, but that would complicate notifying consumers about resizing
and would require some special-casing in the attribute code.

 +static void
 +g_resize_provider_event(void *arg, int flag)
 +{
 [...]
 +if (flag == EV_CANCEL)
 +return;
 
 How it can be canceled? Because I'm clearly missing something. You post
 this event without giving any pointers, so how g_cancel_event() can find
 this event can cancel it?

Copy-pasto, my bad.  Thanks for spotting this.

 +hh = arg;
 +pp = hh-pp;
 +size = hh-size;
 
 Where do you free the memory allocated for 'hh'?

It should be here, and it will be added soon.

 +G_VALID_PROVIDER(pp);
 
 Is this your protection from a provider going away?

Can you suggest a way to do it in a safe way that doesn't involve
rewriting most of GEOM?

 +LIST_FOREACH_SAFE(cp, pp-consumers, consumers, cp2) {
 +gp = cp-geom;
 +if (gp-resize == NULL  size  pp-mediasize)
 +cp-geom-orphan(cp);
 +}
 
 Why is this safe to call the orphan method directly and not use
 g_orphan_provider()? I don't know if using g_orphan_provider() is safe
 to use here either, but I'm under impression that you assume no orphan
 method will ever drop the topology lock? We have tools to assert that,
 no need to introduce such weak assumptions.

It's not that using g_orphan_provider() would be safer here - it simply
wouldn't work.  The way it works is by adding providers to a queue
(g_doorstep).  _Providers_, and we need to orphan individual consumers.
So, this would involve rewriting the orphanisation mechanism.  Also,
most of the classes were fixed by mav@ to handle this correctly, IIRC.

 +void
 +g_resize_provider(struct g_provider *pp, off_t size)
 +{
 +struct g_hh00 *hh;
 +
 +G_VALID_PROVIDER(pp);
 +
 +if (size == pp-mediasize)
 +return;
 +
 +hh = g_malloc(sizeof *hh, M_WAITOK | M_ZERO);
 +hh-pp = pp;
 
 Care to explain why the provider can't disappear between now and the
 event thread calling g_resize_provider_event()?

See above.

-- 
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r235918 - head/sys/geom/label

[Edward Tomasz Napierała]

Wiadomość napisana przez Andriy Gapon w dniu 25 maj 2012, o godz. 09:20:

 on 24/05/2012 19:48 Edward Tomasz Napierala said the following:
 Author: trasz
 Date: Thu May 24 16:48:33 2012
 New Revision: 235918
 URL: http://svn.freebsd.org/changeset/base/235918
 
 Log:
  Make g_label(4) ignore provider size when looking for UFS labels.
  Without it, it fails to create labels for filesystems resized by
  growfs(8).
 
  PR: kern/165962
  Submitted by:   Olivier Cochard-Labbe olivier at cochard dot me
 
 Was this change discussed somewhere?  Reviewed even?
 
 I was once curious why the size check was there and there was a very valid
 reason to have it:
 http://lists.freebsd.org/pipermail/freebsd-geom/2009-April/003473.html
 Has anything changed?

Nope, I just didn't investigate it enough.  I'm testing a fix by Jung-uk Kim;
if it works, I'll commit it, otherwise I'll revert it.  In any case, I'll
add a comment describing why it's there.  Sorry for the breakage.

-- 
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r228843 - head/contrib/telnet/libtelnet head/crypto/heimdal/appl/telnet/libtelnet head/include head/lib/libc/gen head/lib/libc/iconv head/lib/libc/include head/lib/libc/net head/libexe

[Edward Tomasz Napierała]

Wiadomość napisana przez Andrey Chernov w dniu 24 gru 2011, o godz. 11:50:
 On Sat, Dec 24, 2011 at 02:45:21AM -0800, Xin LI wrote:
 On Sat, Dec 24, 2011 at 2:39 AM, Andrey Chernov a...@freebsd.org wrote:
 On Sat, Dec 24, 2011 at 02:26:20AM -0800, Xin LI wrote:
 chroot(2) can create legitimate and secure environment where dlopen(2)
 is safe and necessary.
 
 Yes, so ischroot() check can be used only into that places where libc's
 libc_dlopen() currently used, i.e. placed into libc_dlopen() itself.
 
 So it's Okay to break NSS in chroot jail?
 
 We need general solution. We simple can't count all possible and future 
 ftpd's arround the world and insert __FreeBSD_libc_enter_restricted_mode() 
 into them. I even not mention other programs that may use chroot() too. If 
 some component like auth is critical for chroot, it should be restricted 
 in general scope.

How about adding a check in dlopen(3) to make sure the file being opened
is owned either by us (getuid(3)) or root and is not writable by anyone 
else?___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r225868 - head/bin/ps

[Edward Tomasz Napierała]

Wiadomość napisana przez Alexander Best w dniu 29 wrz 2011, o godz. 20:57:
 On Thu Sep 29 11, Edward Tomasz Napiera?a wrote:
 Wiadomo?? napisana przez Alexander Best w dniu 29 wrz 2011, o godz. 14:14:
 On Thu Sep 29 11, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Thu Sep 29 06:31:42 2011
 New Revision: 225868
 URL: http://svn.freebsd.org/changeset/base/225868
 
 Log:
 Make ps(1) automatically size its column widths.
 
 cool to see this committed. :)
 
 i think i wrote you about the two spaces between the TT and TIME field
 some time ago. you said that the TT entry can contain a postfix -. can
 you describe a situation, where this can occur? i've never seen the TT 
 field
 contain an entry larger than 2 chars.
 
 Login via ssh, run nohup sleep 1000 , then logout.
 
 maybe it would also be possible to introduce unit symbols for the vsz and
 rss fields. they are getting really huge on systems with a long uptime
 (*cough* mem leaks) and kilobyte granularity isn't really that great when
 those fields are nearing 1GB.

I thought about it, but the -h option is already taken.

--
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r225868 - head/bin/ps

[Edward Tomasz Napierała]

Wiadomość napisana przez Alexander Best w dniu 29 wrz 2011, o godz. 14:14:
 On Thu Sep 29 11, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Thu Sep 29 06:31:42 2011
 New Revision: 225868
 URL: http://svn.freebsd.org/changeset/base/225868
 
 Log:
  Make ps(1) automatically size its column widths.
 
 cool to see this committed. :)
 
 i think i wrote you about the two spaces between the TT and TIME field
 some time ago. you said that the TT entry can contain a postfix -. can
 you describe a situation, where this can occur? i've never seen the TT field
 contain an entry larger than 2 chars.

Login via ssh, run nohup sleep 1000 , then logout.

--
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r220387 - head/sys/vm

[Edward Tomasz Napierała]

Wiadomość napisana przez Garrett Cooper w dniu 2011-04-06, o godz. 18:57:
 On Wed, Apr 6, 2011 at 9:27 AM, Edward Tomasz Napierala
 tr...@freebsd.org wrote:
 Author: trasz
 Date: Wed Apr  6 16:27:04 2011
 New Revision: 220387
 URL: http://svn.freebsd.org/changeset/base/220387
 
 Log:
  In vm_daemon(), do not skip processes stopped with SIGSTOP.
 
Did you run this by anyone else before you committed the change?

The whole racct patchset was reviewed by kib@, and I seem to remember
that he said this might cause problems.  However, I didn't encounter
any problems with this, neither did any person testing the patchset.

So, what's wrong with this?

--
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r220168 - head/etc

[Edward Tomasz Napierała]

Wiadomość napisana przez Gavin Atkinson w dniu 2011-03-31, o godz. 11:34:
 On Wed, 2011-03-30 at 18:35 +, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Wed Mar 30 18:35:02 2011
 New Revision: 220168
 URL: http://svn.freebsd.org/changeset/base/220168
 
 Log:
  Add example devd.conf entry.
 
 +
 +# This example works around a memory leak in PostgreSQL, restarting
 +# it when user:pgsql:swap:devctl=1G rctl(8) rule gets triggered.
 +notify 0 {
 +match system  RCTL;
 +match ruleuser:70:swap:.*;
 +action  /usr/local/etc/rc.d/postgresql restart
 +};
 +
 */
 
 This seems like a dangerous rule to have enabled by default?

Yes.  Note, however, the */ in the last line above - the whole block
is commented out.

--
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r217048 - head/share/man/man9

[Edward Tomasz Napierała]

Wiadomość napisana przez Julian Elischer w dniu 2011-01-06, o godz. 10:02:
 On 1/6/11 12:33 AM, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Thu Jan  6 08:33:48 2011
 New Revision: 217048
 URL: http://svn.freebsd.org/changeset/base/217048
 
 Log:
   Get rid of bad advice regarding /* NOTREACHED */.  Compilers don't
   really need it (one can use __dead2 instead), and style(9) was not
   even consistent with itself in this regard.
 
 Modified:
   head/share/man/man9/style.9
 
 Modified: head/share/man/man9/style.9
 ==
 --- head/share/man/man9/style.9  Thu Jan  6 08:13:30 2011
 (r217047)
 +++ head/share/man/man9/style.9  Thu Jan  6 08:33:48 2011
 (r217048)
 @@ -470,9 +470,6 @@ statement that cascade should have a
  .Li FALLTHROUGH
  comment.
  Numerical arguments should be checked for accuracy.
 -Code that cannot be reached should have a
 -.Li NOTREACHED
 -comment.
 
 I object STRONGLY to this change.
 
 The NOTREACHED is also to help the reader understand what has happened and as 
 an afterthought
 was also useful in LINT.  I know know of no technological change to the 
 average reader that makes
 it less useful.

What about adding something like this?

Code that cannot be reached for non-obvious reasons may have
.Li NOTREACHED
comment.

--
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r209046 - stable/8/lib/libc/posix1e

[Edward Tomasz Napierała]

Wiadomość napisana przez Edward Tomasz Napierala w dniu 2010-06-11, o godz. 
17:21:
 Author: trasz
 Date: Fri Jun 11 15:21:12 2010
 New Revision: 209046
 URL: http://svn.freebsd.org/changeset/base/209046
 
 Log:

MFC r208784:

  Fix usage of uninitialized variable.
 
  Found with:  Coverity Prevent
  CID: 7517
  Approved by: re (kib)
 
 Modified:
  stable/8/lib/libc/posix1e/acl_strip.c

[..]

--
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r203721 - head/share/man/man9

[Edward Tomasz Napierała]

Wiadomość napisana przez John Baldwin w dniu 2010-02-09, o godz. 22:16:
 On Tuesday 09 February 2010 3:58:39 pm Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Tue Feb  9 20:58:39 2010
 New Revision: 203721
 URL: http://svn.freebsd.org/changeset/base/203721
 
 Log:
  Add references to VOP_* man pages to vnode(9).
 
 Hmm, it seems VOP_MARKATIME.9 is missing, probably because the actual manpage 
 for that is missing. :-P

Same situation with VOP_CACHEDLOOKUP, VOP_WHITEOUT, VOP_MARKATIME, VOP_POLL,
VOP_KQFILTER, VOP_BMAP, VOP_GETWRITEMOUNT, VOP_ADVLOCKASYNC and VOP_SETLABEL.

--
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r203264 - stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

[Edward Tomasz Napierała]

Wiadomość napisana przez Edward Tomasz Napierala w dniu 2010-01-31, o godz. 
03:11:
 Author: trasz
 Date: Sun Jan 31 02:11:14 2010
 New Revision: 203264
 URL: http://svn.freebsd.org/changeset/base/203264

Note that MFC of NFSv4 ACL support in userland and ZFS was:

Sponsored by:   bytecamp GmbH (http://bytecamp.net)

Patch against 8.0-RELEASE can be found at:

http://people.freebsd.org/~trasz/acl-zfs-8.0.diff

--
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r201794 - in head/sys: ddb dev/ep dev/ex netinet6

[Edward Tomasz Napierała]

Wiadomość napisana przez Stefan Farfeleder w dniu 2010-01-08, o godz. 17:12:
 On Fri, Jan 08, 2010 at 03:44:49PM +, Edward Tomasz Napierala wrote:
 Author: trasz
 Date: Fri Jan  8 15:44:49 2010
 New Revision: 201794
 URL: http://svn.freebsd.org/changeset/base/201794
 
 Log:
  Replace several instances of 'if (!a  b)' with 'if (!(a b))' in order
  to silence newer GCC versions.
 
 They are not identical, !a  b is parsed as (!a)  b.  The code now
 seems more correct however.

Looks like I've got the operator precedence backwards there - I assumed
there was no functional change involved.  But yes, the code now looks more
correct - logical 'and' of a constant with the truth value is pointless.

--
If you cut off my head, what would I say?  Me and my head, or me and my body?

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r198874 - head/sys/kern

[Edward Tomasz Napierała]

Wiadomość napisana przez Pawel Jakub Dawidek w dniu 2009-11-04, o  
godz. 08:04:
On Wed, Nov 04, 2009 at 06:48:34AM +, Edward Tomasz Napierala  
wrote:

Author: trasz
Date: Wed Nov  4 06:48:34 2009
New Revision: 198874
URL: http://svn.freebsd.org/changeset/base/198874

Log:
 Make sure we don't end up with VAPPEND without VWRITE, if someone  
calls open(2)

 like this: open(..., O_APPEND).

Modified:
 head/sys/kern/vfs_vnops.c

Modified: head/sys/kern/vfs_vnops.c
= 
= 
= 
= 
= 
= 
= 
= 
= 
=

--- head/sys/kern/vfs_vnops.c   Wed Nov  4 06:47:14 2009(r198873)
+++ head/sys/kern/vfs_vnops.c   Wed Nov  4 06:48:34 2009(r198874)
@@ -213,7 +213,7 @@ restart:
if (fmode  FEXEC)
accmode |= VEXEC;
if (fmode  O_APPEND)
-   accmode |= VAPPEND;
+   accmode |= VWRITE | VAPPEND;
#ifdef MAC
error = mac_vnode_check_open(cred, vp, accmode);
if (error)


Why? If someone does O_APPEND only we don't want to give him write
access...


As it is now, VAPPEND is not a real V* flag - it's a kind of modifier  
to VWRITE.
Which means that it doesn't really make sense, from the conceptual  
point of view,
to have VAPPEND without VWRITE being set at the same time.  This  
doesn't break
things right now - at least I don't know about any such breakage - but  
in the
future I'd like to have a few KASSERTs to verify that VAPPEND is never  
specified
without VWRITE, just to make sure something unexpected doesn't happen  
somewhere.


As it is now, doing open(..., O_APPEND) will result in a  
filedescriptor open
for... reading.  So, the change above would change the behaviour.   
What about

something like this instead:

Index: vfs_vnops.c
===
--- vfs_vnops.c (revision 198876)
+++ vfs_vnops.c (working copy)
@@ -212,7 +212,7 @@
accmode |= VREAD;
if (fmode  FEXEC)
accmode |= VEXEC;
-   if (fmode  O_APPEND)
+   if ((fmode  O_APPEND)  (fmode  FWRITE))
accmode |= VAPPEND;
 #ifdef MAC
error = mac_vnode_check_open(cred, vp, accmode);

--
If you cut off my head, what would I say?  Me and my head, or me and  
my body?


___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org