Re: CFR: extend use of nitems() macro in the kernel.

[Conrad Meyer]

On Sat, Apr 16, 2016 at 11:25 AM, Pedro Giffuni  wrote:
> Hello;
>
> Using coccinelle, and some hand re-formatting, I generated a patch to
> make use of the nitems() macro in sys, which is too big for
> phabricator [1].
>
> I was careful to exclude anything from the contrib directory or
> any code that is shared with userland (as to not have to include
> additional headers).
>
> The patch is big but pretty safe, I think. The changes are small but
> still it touches many files[1].
>
> I would like some feedback on the convenience of doing such replacement.
> If it is a good idea we could do the same for roundup2() and, in fact,
> I think DragonFly has already done this.

Assuming this change is correct, I am all for it.

Assuming roundup2()s can be cleaned up in an automated and correct
way, I am all for that sort of mechanical change too.

Best,
Conrad
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

CFR: extend use of nitems() macro in the kernel.

[Pedro Giffuni]

Hello;

Using coccinelle, and some hand re-formatting, I generated a patch to
make use of the nitems() macro in sys, which is too big for
phabricator [1].

I was careful to exclude anything from the contrib directory or
any code that is shared with userland (as to not have to include
additional headers).

The patch is big but pretty safe, I think. The changes are small but
still it touches many files[1].

I would like some feedback on the convenience of doing such replacement.
If it is a good idea we could do the same for roundup2() and, in fact,
I think DragonFly has already done this.

Regards,

Pedro.

[1] https://people.freebsd.org/~pfg/patches/sys-nitems.diff

[2] For those too lazy to check [1], here is a list of affected files.

M   sys/amd64/amd64/amd64_mem.c
M   sys/amd64/amd64/machdep.c
M   sys/amd64/linux/linux_sysvec.c
M   sys/amd64/linux32/linux32_sysvec.c
M   sys/arm/amlogic/aml8726/aml8726_clkmsr.c
M   sys/arm/arm/db_interface.c
M   sys/arm/at91/at91_pmc.c
M   sys/arm/mv/armadaxp/armadaxp.c
M   sys/arm/ti/cpsw/if_cpsw.c
M   sys/arm/xscale/ixp425/ixp425.c
M   sys/boot/common/part.c
M   sys/boot/efi/loader/bootinfo.c
M   sys/boot/mips/beri/boot2/boot2.c
M   sys/boot/uboot/common/metadata.c
M   sys/cam/ata/ata_da.c
M   sys/cam/ata/ata_xpt.c
M   sys/cam/cam.c
M   sys/cam/scsi/scsi_all.c
M   sys/cam/scsi/scsi_cd.c
M   sys/cam/scsi/scsi_da.c
M   sys/cam/scsi/scsi_sa.c
M   sys/cam/scsi/scsi_xpt.c
M   sys/cam/scsi/smp_all.c
M   sys/compat/linux/linux_socket.c
M   sys/ddb/db_variables.c
M   sys/dev/adb/adb_kbd.c
M   sys/dev/advansys/adv_isa.c
M   sys/dev/advansys/advlib.c
M   sys/dev/advansys/adw_pci.c
M   sys/dev/advansys/adwlib.c
M   sys/dev/ae/if_ae.c
M   sys/dev/age/if_age.c
M   sys/dev/agp/agp.c
M   sys/dev/agp/agp_ali.c
M   sys/dev/agp/agp_amd64.c
M   sys/dev/aha/aha_isa.c
M   sys/dev/aic/aic.c
M   sys/dev/aic/aic_cbus.c
M   sys/dev/aic/aic_isa.c
M   sys/dev/ale/if_ale.c
M   sys/dev/altera/atse/if_atse.c
M   sys/dev/atkbdc/atkbd.c
M   sys/dev/atkbdc/atkbdc.c
M   sys/dev/atkbdc/psm.c
M   sys/dev/bktr/bktr_core.c
M   sys/dev/bwi/bwirf.c
M   sys/dev/bwn/if_bwn.c
M   sys/dev/cardbus/cardbus_cis.c
M   sys/dev/digi/digi.c
M   sys/dev/digi/digi_isa.c
M   sys/dev/dwc/if_dwc.c
M   sys/dev/ed/if_ed_hpp.c
M   sys/dev/ed/if_ed_isa.c
M   sys/dev/ed/if_ed_pci.c
M   sys/dev/fb/creator.c
M   sys/dev/fb/fb.c
M   sys/dev/fb/machfb.c
M   sys/dev/fb/vesa.c
M   sys/dev/fb/vga.c
M   sys/dev/flash/mx25l.c
M   sys/dev/hatm/if_hatm.c
M   sys/dev/hifn/hifn7751.c
M   sys/dev/hwpmc/hwpmc_amd.c
M   sys/dev/hwpmc/hwpmc_core.c
M   sys/dev/hwpmc/hwpmc_e500.c
M   sys/dev/hwpmc/hwpmc_mips24k.c
M   sys/dev/hwpmc/hwpmc_mips74k.c
M   sys/dev/hwpmc/hwpmc_mpc7xxx.c
M   sys/dev/hwpmc/hwpmc_octeon.c
M   sys/dev/hwpmc/hwpmc_uncore.c
M   sys/dev/hwpmc/hwpmc_xscale.c
M   sys/dev/if_ndis/if_ndis.c
M   sys/dev/jme/if_jme.c
M   sys/dev/kbd/kbd.c
M   sys/dev/le/if_le_isa.c
M   sys/dev/le/if_le_lebuffer.c
M   sys/dev/le/if_le_ledma.c
M   sys/dev/mlx/mlx.c
M   sys/dev/mxge/if_mxge.c
M   sys/dev/nand/nand_id.c
M   sys/dev/ncr/ncr.c
M   sys/dev/nctgpio/nctgpio.c
M   sys/dev/nfe/if_nfe.c
M   sys/dev/patm/if_patm_attach.c
M   sys/dev/pccard/pccard_cis_quirks.c
M   sys/dev/rc/rc.c
M   sys/dev/re/if_re.c
M   sys/dev/rl/if_rl.c
M   sys/dev/rndtest/rndtest.c
M   sys/dev/sf/if_sf.c
M   sys/dev/sge/if_sge.c
M   sys/dev/siba/siba.c
M   sys/dev/sio/sio.c
M   sys/dev/sound/isa/gusc.c
M   sys/dev/sound/pci/emu10kx.c
M   sys/dev/speaker/spkr.c
M   sys/dev/stge/if_stge.c
M   sys/dev/uart/uart_kbd_sun.c
M   sys/dev/uart/uart_subr.c
M   sys/dev/usb/input/ukbd.c
M   sys/dev/usb/serial/u3g.c
M   sys/dev/usb/serial/uchcom.c
M   sys/dev/usb/serial/umcs.c
M   sys/dev/usb/serial/uplcom.c
M   sys/dev/vkbd/vkbd.c
M   sys/dev/wbwd/wbwd.c
M   sys/fs/autofs/autofs.c
M   sys/fs/nfs/nfs_commonkrpc.c
M   sys/geom/part/g_part_bsd.c
M   sys/geom/part/g_part_ebr.c
M   sys/geom/part/g_part_ldm.c
M   sys/geom/part/g_part_mbr.c
M   sys/i386/i386/i686_mem.c
M   sys/i386/i386/machdep.c
M   sys/i386/ibcs2/ibcs2_sysvec.c
M   sys/i386/linux/linux_sysvec.c
M   sys/kern/kern_dump.c
M   sys/kern/kern_ffclock.c
M   sys/kern/kern_jail.c
M   sys/kern/kern_ktrace.c
M   sys/kern/subr_hash.c
M   sys/kern/subr_witness.c
M   sys/kern/sysv_msg.c
M   sys/kern/sysv_sem.c
M   sys/kern/uipc_usrreq.c
M   sys/mips/mips/db_interface.c
M   sys/mips/nlm/board.c
M   sys/mips/nlm/xlp_machdep.c
M   sys/mips/rmi/dev/nlge/if_nlge.c
M   sys/net/netisr.c
M   sys/net/rtsock.c
M   

Re: Heads up

[O. Hartmann]

Am Fri, 15 Apr 2016 08:26:23 -0600
Warner Losh  schrieb:

> On Fri, Apr 15, 2016 at 7:51 AM, O. Hartmann 
> wrote:
> 
> > On Thu, 14 Apr 2016 22:19:23 -0600
> > Warner Losh  wrote:
> >  
> > > On Thu, Apr 14, 2016 at 9:56 PM, Warren Block   
> > wrote:  
> > >  
> > > > On Thu, 14 Apr 2016, Warner Losh wrote:
> > > >
> > > > The CAM I/O scheduler has been committed to current. This work is
> > > > described  
> > > >> in https://people.freebsd.org/~imp/bsdcan2015/iosched-v3.pdf though  
> > the  
> > > >> default scheduler doesn't change the default (old) behavior.
> > > >>
> > > >> One possible issue, however, is that it also enables NCQ Trims on ada
> > > >> SSDs.
> > > >> There are a few rogue drives that claim support for this feature, but
> > > >> actually implement data corrupt instead of queued trims. The list of  
> > known  
> > > >> rogues is believed to be complete, but some caution is in order.  
> > > >
> > > >  
> > >  
> > > > Is the list of drives queryable?  Is there an easy way to tell if the
> > > > currently-connected drives are on the list?
> > > >  
> > >
> > > /usr/src/sys/cam/ata/ata_da.c has the list.
> > >
> > > dmesg will tell you if it detected a bad one since it prints the drive's
> > > quirks.
> > > But that's no big deal, because the bad one work just fine if you never
> > > issue
> > > a NCQ TRIM. This small group of drives were early adapters of this
> > > technology
> > >
> > > Here's the full list of known rogues:
> > >
> > > Crucial/Micron M500 (all firmware prior to MU07)
> > > Micron M510 MU01 firmware (newer firmware is good)
> > > Crucial/Micron M550 MU01 firmware (newer firmware is good)
> > > Crucial MX100 MU01 firmware (newer firmware is good)
> > > FCCT M500 all firmware
> > > Samsung 830 all firmware
> > > Samsung 840 all firmware
> > > Samsung 850 all firmware  
> >
> > This is funny,
> >
> > ALL of our Fujitsu Workstations and those I use pprivate do have Micron SSD
> > drives (Fujitsu) and Samsung SSD (830 and 840).
> >  
> 
> Which Micron drives? I'm not familiar with the 'Fujitsu' model for that
> line.

The boxes are Celsius M-7XX series boxes, I checked but forgot, but I think its 
M600 type
SSDs (sold with workstations Marsch/April 2015).

> The newer ones are fine, the ones listed above with the MU01 firmware
> being bad have MU02 firmware (or newer) available that fixes the problem.
> The M500's firmware exists, but I don't know how available it is. So for
> Micron, solutions to the problem exist.
> 
> The 830 and 840 apparently claim support, but are in the class of drives
> that simply fail to actually work in what may be a reasonable way to detect.
> There's code to do the fallback in there now, but I'm not so sure that it is
> working given some of the reports I've seen. Maybe I need to try to but
> a couple of these drives to see.

I was mistaken, I personally use some 830 and since a couple of weeks 850 (not 
840). Both
are marked as you described.

> 
> So - they are the most popular/used drives in a more "professional"
> > environment these days and they get blacklisted :-(
> >  
> 
> At least for the micron drives, you don't want to use NCQ trim on the
> versions listed. It's data corruption roulette. For newer drives, it is
> fine.
> The performance will certainly be no worse than it was before my
> commit.
> 
> Warner
> ___
> freebsd-current@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"



pgpq2IZXyPZSi.pgp
Description: OpenPGP digital signature

Re: Heads up

[O. Hartmann]

Am Fri, 15 Apr 2016 11:30:05 -0600
Warner Losh  schrieb:

> On Fri, Apr 15, 2016 at 11:25 AM, Maxim Sobolev  wrote:
> 
> > Conrad, yes, you can, but sticking it into FreeBSD source tree IMHO
> > restricts your options somewhat. If it's your private code you can easily
> > put F-world into it and nobody obviously cares apart from your co-workers
> > and your boss. Would probably be considered highly inappropriate by most to
> > merge it into a public one, so "whatever your like" rule has its limits.
> >  
> 
> The name isn't obscene or offensive.
> 
> 
> > Also, apart from being not very suggestive as to what this option actually
> > does, let's not forget that "Netflix" is probably registered as a trademark
> > and whatnot in North America at least, if not worldwide. I am not a lawyer,
> > but I suspect using it might have some ramifications for our ability to
> > name things using this name.
> >  
> 
> Probably not. This is a weak argument. It's a descriptive use of the word,
> and not being used to market or sell video streaming services or DVD rental
> by mail.
> 
> 
> > If "SSDNG" is not a good name, I'd suggest coming up with some other name
> > that is neutral and also has some hints as to its functionality. For
> > example, CAM_IOCHED_LOW_LAT or something.
> >  
> 
> Also a horrible name. It's a generic I/O scheduler. It can do lots of
> things. I keep saying that, and categorically refuse to name the more
> expansive scheduler anything that's so limiting.
> 
> Warner

Why the not simply CAM_IOSCHED_EXT for an extension of the already present i/o
scheduling? Or without the _EXT?

> 
> 
> > On Fri, Apr 15, 2016 at 9:22 AM, Conrad Meyer  wrote:
> >  
> >> Max,
> >>
> >> If you implement a new IO scheduler you can name it whatever you like.
> >> "NG" isn't any more meaningful than "Netflix."
> >>
> >> Best,
> >> Conrad
> >>
> >> On Fri, Apr 15, 2016 at 9:13 AM, Maxim Sobolev 
> >> wrote:  
> >> > Great, work Warner, thanks! Small note, though. The CAM_IOSCHED_NETFLIX
> >> > seems like a quite poor name for a kernel option. IMHO there is no good
> >> > reason for polluting it with the name of the company that sponsored the
> >> > development. I don't think we have any precedents of doing this unless  
> >> the  
> >> > option is related to a piece of hardware that the company makes, and  
> >> it's  
> >> > not the case here. Apart from "coolness" factor as far as I understand  
> >> that  
> >> > _NETFLIX suffix does not give any tangible benefit for anybody reading
> >> > kernel config and trying to understand what this option actually does.
> >> > CAM_IOSCHED_SSDNG or something would be better IMHO. Just my $0.02.
> >> >
> >> > -Max
> >> >
> >> > On Thu, Apr 14, 2016 at 3:42 PM, Warner Losh  wrote:
> >> >  
> >> >> The CAM I/O scheduler has been committed to current. This work is  
> >> described  
> >> >> in https://people.freebsd.org/~imp/bsdcan2015/iosched-v3.pdf though  
> >> the  
> >> >> default scheduler doesn't change the default (old) behavior.
> >> >>
> >> >> One possible issue, however, is that it also enables NCQ Trims on ada  
> >> SSDs.  
> >> >> There are a few rogue drives that claim support for this feature, but
> >> >> actually implement data corrupt instead of queued trims. The list of  
> >> known  
> >> >> rogues is believed to be complete, but some caution is in order.
> >> >>
> >> >> Warner
> >> >> ___
> >> >> freebsd-current@freebsd.org mailing list
> >> >> https://lists.freebsd.org/mailman/listinfo/freebsd-current
> >> >> To unsubscribe, send any mail to "  
> >> freebsd-current-unsubscr...@freebsd.org"  
> >> >>
> >> >>  
> >> > ___
> >> > freebsd-current@freebsd.org mailing list
> >> > https://lists.freebsd.org/mailman/listinfo/freebsd-current
> >> > To unsubscribe, send any mail to "  
> >> freebsd-current-unsubscr...@freebsd.org"
> >>
> >>  
> >  
> ___
> freebsd-current@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"



pgpcaVjbfUSl2.pgp
Description: OpenPGP digital signature