| Message ID | 1333124074-19205-1-git-send-email-james.leddy@canonical.com |
|---|---|
| State | New |
| Headers | show |
On 03/30/2012 10:14 AM, James M Leddy wrote: > From: Asai Thambi S P <asamymuthupa@micron.com> > > Removed the following: > * irrelevant argument 'barrier' of mtip_hw_submit_io() > * unused member 'eh_active' of struct driver_data > > Signed-off-by: Asai Thambi S P <asamymuthupa@micron.com> > Signed-off-by: Sam Bradshaw <sbradshaw@micron.com> > Signed-off-by: Jens Axboe <axboe@kernel.dk> > (cherry picked from commit 4e8670e26135d8fbfd5e084fddc1a8ed9f8eb4cb) > > From the merge: > > - Killing the barrier part of mtip32xx. It doesn't really support > barriers, and it doesn't need them (writes are fully ordered). > > Follow on from mtip32xx support that was added in 3.2.0-13.22. > I have compile tested. > > Signed-off-by: James M Leddy <james.leddy@canonical.com> > --- > drivers/block/mtip32xx/mtip32xx.c | 11 +++++------ > drivers/block/mtip32xx/mtip32xx.h | 5 ----- > 2 files changed, 5 insertions(+), 11 deletions(-) > > diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c > index b74eab7..8eb81c9 100644 > --- a/drivers/block/mtip32xx/mtip32xx.c > +++ b/drivers/block/mtip32xx/mtip32xx.c > @@ -2068,8 +2068,6 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd, > * when the read completes. > * @data Callback data passed to the callback function > * when the read completes. > - * @barrier If non-zero, this command must be completed before > - * issuing any other commands. > * @dir Direction (read or write) > * > * return value > @@ -2077,7 +2075,7 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd, > */ > static void mtip_hw_submit_io(struct driver_data *dd, sector_t start, > int nsect, int nents, int tag, void *callback, > - void *data, int barrier, int dir) > + void *data, int dir) > { > struct host_to_dev_fis *fis; > struct mtip_port *port = dd->port; > @@ -2108,8 +2106,6 @@ static void mtip_hw_submit_io(struct driver_data *dd, sector_t start, > *((unsigned int *) &fis->lba_low) = (start & 0xFFFFFF); > *((unsigned int *) &fis->lba_low_ex) = ((start >> 24) & 0xFFFFFF); > fis->device = 1 << 6; > - if (barrier) > - fis->device |= FUA_BIT; > fis->features = nsect & 0xFF; > fis->features_ex = (nsect >> 8) & 0xFF; > fis->sect_count = ((tag << 3) | (tag >> 5)); > @@ -3087,7 +3083,6 @@ static void mtip_make_request(struct request_queue *queue, struct bio *bio) > tag, > bio_endio, > bio, > - bio->bi_rw & REQ_FUA, > bio_data_dir(bio)); > } else > bio_io_error(bio); > @@ -3187,6 +3182,10 @@ skip_create_disk: > blk_queue_max_segments(dd->queue, MTIP_MAX_SG); > blk_queue_physical_block_size(dd->queue, 4096); > blk_queue_io_min(dd->queue, 4096); > + /* > + * write back cache is not supported in the device. FUA depends on > + * write back cache support, hence setting flush support to zero. > + */ > blk_queue_flush(dd->queue, 0); > > /* Set the capacity of the device in 512 byte sectors. */ > diff --git a/drivers/block/mtip32xx/mtip32xx.h b/drivers/block/mtip32xx/mtip32xx.h > index 723d7c4..e0554a8 100644 > --- a/drivers/block/mtip32xx/mtip32xx.h > +++ b/drivers/block/mtip32xx/mtip32xx.h > @@ -104,9 +104,6 @@ > /* BAR number used to access the HBA registers. */ > #define MTIP_ABAR 5 > > -/* Forced Unit Access Bit */ > -#define FUA_BIT 0x80 > - > #ifdef DEBUG > #define dbg_printk(format, arg...) \ > printk(pr_fmt(format), ##arg); > @@ -415,8 +412,6 @@ struct driver_data { > > atomic_t resumeflag; /* Atomic variable to track suspend/resume */ > > - atomic_t eh_active; /* Flag for error handling tracking */ > - > struct task_struct *mtip_svc_handler; /* task_struct of svc thd */ > }; > Does this have an actual impact on driver performance or functionality ?
On 03/30/2012 01:11 PM, Tim Gardner wrote: > On 03/30/2012 10:14 AM, James M Leddy wrote: >> From: Asai Thambi S P <asamymuthupa@micron.com> >> >> Removed the following: >> * irrelevant argument 'barrier' of mtip_hw_submit_io() >> * unused member 'eh_active' of struct driver_data >> >> Signed-off-by: Asai Thambi S P <asamymuthupa@micron.com> >> Signed-off-by: Sam Bradshaw <sbradshaw@micron.com> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> >> (cherry picked from commit 4e8670e26135d8fbfd5e084fddc1a8ed9f8eb4cb) >> >> From the merge: >> >> - Killing the barrier part of mtip32xx. It doesn't really support >> barriers, and it doesn't need them (writes are fully ordered). >> >> Follow on from mtip32xx support that was added in 3.2.0-13.22. >> I have compile tested. >> >> Signed-off-by: James M Leddy <james.leddy@canonical.com> >> --- >> drivers/block/mtip32xx/mtip32xx.c | 11 +++++------ >> drivers/block/mtip32xx/mtip32xx.h | 5 ----- >> 2 files changed, 5 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c >> index b74eab7..8eb81c9 100644 >> --- a/drivers/block/mtip32xx/mtip32xx.c >> +++ b/drivers/block/mtip32xx/mtip32xx.c >> @@ -2068,8 +2068,6 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd, >> * when the read completes. >> * @data Callback data passed to the callback function >> * when the read completes. >> - * @barrier If non-zero, this command must be completed before >> - * issuing any other commands. >> * @dir Direction (read or write) >> * >> * return value >> @@ -2077,7 +2075,7 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd, >> */ >> static void mtip_hw_submit_io(struct driver_data *dd, sector_t start, >> int nsect, int nents, int tag, void *callback, >> - void *data, int barrier, int dir) >> + void *data, int dir) >> { >> struct host_to_dev_fis *fis; >> struct mtip_port *port = dd->port; >> @@ -2108,8 +2106,6 @@ static void mtip_hw_submit_io(struct driver_data *dd, sector_t start, >> *((unsigned int *) &fis->lba_low) = (start & 0xFFFFFF); >> *((unsigned int *) &fis->lba_low_ex) = ((start >> 24) & 0xFFFFFF); >> fis->device = 1 << 6; >> - if (barrier) >> - fis->device |= FUA_BIT; >> fis->features = nsect & 0xFF; >> fis->features_ex = (nsect >> 8) & 0xFF; >> fis->sect_count = ((tag << 3) | (tag >> 5)); >> @@ -3087,7 +3083,6 @@ static void mtip_make_request(struct request_queue *queue, struct bio *bio) >> tag, >> bio_endio, >> bio, >> - bio->bi_rw & REQ_FUA, >> bio_data_dir(bio)); >> } else >> bio_io_error(bio); >> @@ -3187,6 +3182,10 @@ skip_create_disk: >> blk_queue_max_segments(dd->queue, MTIP_MAX_SG); >> blk_queue_physical_block_size(dd->queue, 4096); >> blk_queue_io_min(dd->queue, 4096); >> + /* >> + * write back cache is not supported in the device. FUA depends on >> + * write back cache support, hence setting flush support to zero. >> + */ >> blk_queue_flush(dd->queue, 0); >> >> /* Set the capacity of the device in 512 byte sectors. */ >> diff --git a/drivers/block/mtip32xx/mtip32xx.h b/drivers/block/mtip32xx/mtip32xx.h >> index 723d7c4..e0554a8 100644 >> --- a/drivers/block/mtip32xx/mtip32xx.h >> +++ b/drivers/block/mtip32xx/mtip32xx.h >> @@ -104,9 +104,6 @@ >> /* BAR number used to access the HBA registers. */ >> #define MTIP_ABAR 5 >> >> -/* Forced Unit Access Bit */ >> -#define FUA_BIT 0x80 >> - >> #ifdef DEBUG >> #define dbg_printk(format, arg...) \ >> printk(pr_fmt(format), ##arg); >> @@ -415,8 +412,6 @@ struct driver_data { >> >> atomic_t resumeflag; /* Atomic variable to track suspend/resume */ >> >> - atomic_t eh_active; /* Flag for error handling tracking */ >> - >> struct task_struct *mtip_svc_handler; /* task_struct of svc thd */ >> }; >> > > Does this have an actual impact on driver performance or functionality ? I'm not sure, I'll ask the reporter.
On 03/30/2012 12:12 PM, James M Leddy wrote: > On 03/30/2012 01:11 PM, Tim Gardner wrote: >> On 03/30/2012 10:14 AM, James M Leddy wrote: >>> From: Asai Thambi S P <asamymuthupa@micron.com> >>> >>> Removed the following: >>> * irrelevant argument 'barrier' of mtip_hw_submit_io() >>> * unused member 'eh_active' of struct driver_data >>> >>> Signed-off-by: Asai Thambi S P <asamymuthupa@micron.com> >>> Signed-off-by: Sam Bradshaw <sbradshaw@micron.com> >>> Signed-off-by: Jens Axboe <axboe@kernel.dk> >>> (cherry picked from commit 4e8670e26135d8fbfd5e084fddc1a8ed9f8eb4cb) >>> >>> From the merge: >>> >>> - Killing the barrier part of mtip32xx. It doesn't really support >>> barriers, and it doesn't need them (writes are fully ordered). >>> >>> Follow on from mtip32xx support that was added in 3.2.0-13.22. >>> I have compile tested. >>> >>> Signed-off-by: James M Leddy <james.leddy@canonical.com> >>> --- >>> drivers/block/mtip32xx/mtip32xx.c | 11 +++++------ >>> drivers/block/mtip32xx/mtip32xx.h | 5 ----- >>> 2 files changed, 5 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c >>> index b74eab7..8eb81c9 100644 >>> --- a/drivers/block/mtip32xx/mtip32xx.c >>> +++ b/drivers/block/mtip32xx/mtip32xx.c >>> @@ -2068,8 +2068,6 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd, >>> * when the read completes. >>> * @data Callback data passed to the callback function >>> * when the read completes. >>> - * @barrier If non-zero, this command must be completed before >>> - * issuing any other commands. >>> * @dir Direction (read or write) >>> * >>> * return value >>> @@ -2077,7 +2075,7 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd, >>> */ >>> static void mtip_hw_submit_io(struct driver_data *dd, sector_t start, >>> int nsect, int nents, int tag, void *callback, >>> - void *data, int barrier, int dir) >>> + void *data, int dir) >>> { >>> struct host_to_dev_fis *fis; >>> struct mtip_port *port = dd->port; >>> @@ -2108,8 +2106,6 @@ static void mtip_hw_submit_io(struct driver_data *dd, sector_t start, >>> *((unsigned int *) &fis->lba_low) = (start & 0xFFFFFF); >>> *((unsigned int *) &fis->lba_low_ex) = ((start >> 24) & 0xFFFFFF); >>> fis->device = 1 << 6; >>> - if (barrier) >>> - fis->device |= FUA_BIT; >>> fis->features = nsect & 0xFF; >>> fis->features_ex = (nsect >> 8) & 0xFF; >>> fis->sect_count = ((tag << 3) | (tag >> 5)); >>> @@ -3087,7 +3083,6 @@ static void mtip_make_request(struct request_queue *queue, struct bio *bio) >>> tag, >>> bio_endio, >>> bio, >>> - bio->bi_rw & REQ_FUA, >>> bio_data_dir(bio)); >>> } else >>> bio_io_error(bio); >>> @@ -3187,6 +3182,10 @@ skip_create_disk: >>> blk_queue_max_segments(dd->queue, MTIP_MAX_SG); >>> blk_queue_physical_block_size(dd->queue, 4096); >>> blk_queue_io_min(dd->queue, 4096); >>> + /* >>> + * write back cache is not supported in the device. FUA depends on >>> + * write back cache support, hence setting flush support to zero. >>> + */ >>> blk_queue_flush(dd->queue, 0); >>> >>> /* Set the capacity of the device in 512 byte sectors. */ >>> diff --git a/drivers/block/mtip32xx/mtip32xx.h b/drivers/block/mtip32xx/mtip32xx.h >>> index 723d7c4..e0554a8 100644 >>> --- a/drivers/block/mtip32xx/mtip32xx.h >>> +++ b/drivers/block/mtip32xx/mtip32xx.h >>> @@ -104,9 +104,6 @@ >>> /* BAR number used to access the HBA registers. */ >>> #define MTIP_ABAR 5 >>> >>> -/* Forced Unit Access Bit */ >>> -#define FUA_BIT 0x80 >>> - >>> #ifdef DEBUG >>> #define dbg_printk(format, arg...) \ >>> printk(pr_fmt(format), ##arg); >>> @@ -415,8 +412,6 @@ struct driver_data { >>> >>> atomic_t resumeflag; /* Atomic variable to track suspend/resume */ >>> >>> - atomic_t eh_active; /* Flag for error handling tracking */ >>> - >>> struct task_struct *mtip_svc_handler; /* task_struct of svc thd */ >>> }; >>> >> >> Does this have an actual impact on driver performance or functionality ? > > I'm not sure, I'll ask the reporter. > Side channel information indicates that this patch has no impact on functionality or performance, and therefore does not fix a bug making it unsuitable as a stable update. NAK
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index b74eab7..8eb81c9 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -2068,8 +2068,6 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd, * when the read completes. * @data Callback data passed to the callback function * when the read completes. - * @barrier If non-zero, this command must be completed before - * issuing any other commands. * @dir Direction (read or write) * * return value @@ -2077,7 +2075,7 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd, */ static void mtip_hw_submit_io(struct driver_data *dd, sector_t start, int nsect, int nents, int tag, void *callback, - void *data, int barrier, int dir) + void *data, int dir) { struct host_to_dev_fis *fis; struct mtip_port *port = dd->port; @@ -2108,8 +2106,6 @@ static void mtip_hw_submit_io(struct driver_data *dd, sector_t start, *((unsigned int *) &fis->lba_low) = (start & 0xFFFFFF); *((unsigned int *) &fis->lba_low_ex) = ((start >> 24) & 0xFFFFFF); fis->device = 1 << 6; - if (barrier) - fis->device |= FUA_BIT; fis->features = nsect & 0xFF; fis->features_ex = (nsect >> 8) & 0xFF; fis->sect_count = ((tag << 3) | (tag >> 5)); @@ -3087,7 +3083,6 @@ static void mtip_make_request(struct request_queue *queue, struct bio *bio) tag, bio_endio, bio, - bio->bi_rw & REQ_FUA, bio_data_dir(bio)); } else bio_io_error(bio); @@ -3187,6 +3182,10 @@ skip_create_disk: blk_queue_max_segments(dd->queue, MTIP_MAX_SG); blk_queue_physical_block_size(dd->queue, 4096); blk_queue_io_min(dd->queue, 4096); + /* + * write back cache is not supported in the device. FUA depends on + * write back cache support, hence setting flush support to zero. + */ blk_queue_flush(dd->queue, 0); /* Set the capacity of the device in 512 byte sectors. */ diff --git a/drivers/block/mtip32xx/mtip32xx.h b/drivers/block/mtip32xx/mtip32xx.h index 723d7c4..e0554a8 100644 --- a/drivers/block/mtip32xx/mtip32xx.h +++ b/drivers/block/mtip32xx/mtip32xx.h @@ -104,9 +104,6 @@ /* BAR number used to access the HBA registers. */ #define MTIP_ABAR 5 -/* Forced Unit Access Bit */ -#define FUA_BIT 0x80 - #ifdef DEBUG #define dbg_printk(format, arg...) \ printk(pr_fmt(format), ##arg); @@ -415,8 +412,6 @@ struct driver_data { atomic_t resumeflag; /* Atomic variable to track suspend/resume */ - atomic_t eh_active; /* Flag for error handling tracking */ - struct task_struct *mtip_svc_handler; /* task_struct of svc thd */ };