lkml.org 
[lkml]   [2004]   [May]   [22]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
DateSat, 22 May 2004 11:44:47 +0200
FromJens Axboe <>
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 13:03    [from the cache]
©2003-2008