lkml.org 
[lkml]   [2004]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 2.6.6-mm5
    On Sat, May 22 2004, hch@infradead.org wrote:
    > > +disk-barrier-core.patch
    > > +disk-barrier-core-tweaks.patch
    > > +disk-barrier-ide.patch
    > > +disk-barrier-ide-symbol-expoprt.patch
    > > +disk-barrier-ide-warning-fix.patch
    > > +disk-barrier-scsi.patch
    > >
    > > Support for IDE and SCSI barriers
    > >
    > > +disk-barrier-dm.patch
    > > +disk-barrier-md.patch
    > >
    > > Via device mapper and raid as well.
    >
    > Some comments on the API and the SCSI part:
    >
    > - issue_flush_fn prototype choice is bad, the request_queue_t argument
    > wile always be disk->queue so it's not needed and only causes
    > confusion.

    Agree, it's mutated into place which is probably the reason for the
    dupe.

    > - issue_flush sounds a little strange to me, what about cache_flush
    > or sync_cache instead?

    Fine with me, I'm notoriously bad at naming.

    > - scsi_drive.issue_flush should take a scsi_device * as first parameter,
    > not struct device * - makes life for bother caller and callee easier.
    > - should probably add a small helper to get the scsi_driver from the
    > gendisk instead of duplicating the code, ala:
    >
    > static inline struct scsi_driver *scsi_disk_driver(struct gendisk *disk)
    > {
    > return *(struct scsi_driver **) disk->private_data;
    > }

    Fine too.

    > - the WCE check should move into sd_sync_cache

    Ditto

    > - NULL scsi_disk can't happen for sd_issue_flush, no need to check,
    > and thus the disctinction of sd_issue_flush vs sd_sync_cache can
    > go and sd_shutdown can simply call the cache flush method.

    Neat, thanks.

    Thanks for the review Christoph!

    --
    Jens Axboe

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:03    [W:0.024 / U:1.440 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site