Re: [PATCH] Use ida_simple for SCSI iSCSI transport session id
On Fri, Feb 12, 2016 at 09:38:53AM -0800, Lee Duncan wrote: > The scsi_transport_iscsi module already uses the ida_simple > routines for managing the target ID, if requested to do > so. This change replaces an ever-increasing atomic integer > that tracks the session ID itself with the ida_simple > family of routines. This means that the session ID > will be reclaimed and can be reused when the session > is freed. > > Note that no maximum is placed on this value, though > user-space currently only seems to use the lower 24-bits. > It seems better to handle this in user space, though, > than to limit the value range for the session ID here. > > Signed-off-by: Lee Duncan Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] "block: make generic_make_request handle arbitrarily sized bios" breaks boot on parisc-linux
On Thu, 2016-02-25 at 11:38 +0800, Ming Lei wrote:
> On Thu, Feb 25, 2016 at 7:28 AM, John David Anglin <
> dave.ang...@bell.net> wrote:
> > On 2016-02-24, at 4:36 PM, Helge Deller wrote:
> >
> > > Maybe Dave has more luck, otherwise I'll continue to try to get
> > > some info.
> >
> > I tried your patch on the commit in linux-block which first failed
> > to boot. As with Helge, the
> > system crashed and no useful data was output on console. I then
> > applied following patch
> > to give some extra segments and tired again:
> >
> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > index b1a2631..b421f03 100644
> > --- a/drivers/scsi/scsi_lib.c
> > +++ b/drivers/scsi/scsi_lib.c
> > @@ -595,6 +595,11 @@ static int scsi_alloc_sgtable(struct
> > scsi_data_buffer *sdb, int nents, bool mq)
> >
> > BUG_ON(!nents);
> >
> > + /* Provide extra entries in case of split. */
> > + nents += 8;
> > + if (nents > SCSI_MAX_SG_SEGMENTS)
> > + nents = SCSI_MAX_SG_SEGMENTS;
> > +
>
> Yeah, this is needed for sake of safety.
>
> > if (mq) {
> > if (nents <= SCSI_MAX_SG_SEGMENTS) {
> > sdb->table.nents = nents;
> >
> > The attached file shows the crash in first boot. The second boot
> > was successful and various output
> > was generated by your check code.
>
> From the following log(just select one simple, and looks all are
> similar) in
> 2nd boot, the bi_phys_segments is figured out as one by block core
> , which is wrong because the max segment size is 64k according to
> your investigation in the below link, but the whole req/bio is
> 192k(4k*48).
>
> http://www.spinics.net/lists/linux-parisc/msg06749.html
>
> Looks weird, it shouldn't have happened because
> blk_bio_segment_split()
> does respect the max segment size limit.
>
> BTW, what is the scsi driver for the device?
>
> blk_rq_map_sg: merge bug: 3 1, extra_len 0, dma_drain 0
> check_bvec: dump bvec for 7e53c5f0(f:2449, t:1)
> 0: 0 4096 245852 7e2c4c40
> 1: 0 4096 245853 7e2c4c40
> 2: 0 4096 245854 7e2c4c40
> 3: 0 4096 245855 7e2c4c40
> 4: 0 4096 245856 7e2c4c40
> 5: 0 4096 245857 7e2c4c40
> 6: 0 4096 245858 7e2c4c40
> 7: 0 4096 245859 7e2c4c40
> 8: 0 4096 245860 7e2c4c40
> 9: 0 4096 245861 7e2c4c40
>10: 0 4096 245862 7e2c4c40
>11: 0 4096 245863 7e2c4c40
>12: 0 4096 245864 7e2c4c40
>13: 0 4096 245865 7e2c4c40
>14: 0 4096 245866 7e2c4c40
>15: 0 4096 245867 7e2c4c40
>16: 0 4096 245868 7e2c4c40
>17: 0 4096 245869 7e2c4c40
>18: 0 4096 245870 7e2c4c40
>19: 0 4096 245871 7e2c4c40
>20: 0 4096 245872 7e2c4c40
>21: 0 4096 245873 7e2c4c40
>22: 0 4096 245874 7e2c4c40
>23: 0 4096 245875 7e2c4c40
>24: 0 4096 245876 7e2c4c40
>25: 0 4096 245877 7e2c4c40
>26: 0 4096 245878 7e2c4c40
>27: 0 4096 245879 7e2c4c40
>28: 0 4096 245880 7e2c4c40
>29: 0 4096 245881 7e2c4c40
>30: 0 4096 245882 7e2c4c40
>31: 0 4096 245883 7e2c4c40
>32: 0 4096 245884 7e2c4c40
>33: 0 4096 245885 7e2c4c40
>34: 0 4096 245886 7e2c4c40
>35: 0 4096 245887 7e2c4c40
>36: 0 4096 245888 7e2c4c40
>37: 0 4096 245889 7e2c4c40
>38: 0 4096 245890 7e2c4c40
>39: 0 4096 245891 7e2c4c40
>40: 0 4096 245892 7e2c4c40
>41: 0 4096 245893 7e2c4c40
>42: 0 4096 245894 7e2c4c40
>43: 0 4096 245895 7e2c4c40
>44: 0 4096 245896 7e2c4c40
>45: 0 4096 245897 7e2c4c40
>46: 0 4096 245898 7e2c4c40
>47: 0 4096 245899 7e2c4c40
We've provided all the information you asked for, what's the next step
on this, or do we have to unwind the bio splitting code with reverts
until it starts working?
Thanks,
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] brd: thin provisioning support
Implement module parameter 'rd_mem_size' to restrict the overall
allocated memory size and 'rd_lowat_thresh' to add a low water mark
signalling.
Signed-off-by: Hannes Reinecke
---
drivers/block/brd.c | 53 +
1 file changed, 53 insertions(+)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index cb27190..a8848c3 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -30,6 +30,13 @@
#define PAGE_SECTORS (1 << PAGE_SECTORS_SHIFT)
/*
+ * Thin provisioning support
+ */
+static unsigned int rd_total_pages;
+static int rd_mem_size = CONFIG_BLK_DEV_RAM_SIZE * CONFIG_BLK_DEV_RAM_COUNT;
+static int rd_lowat_thresh = CONFIG_BLK_DEV_RAM_SIZE *
CONFIG_BLK_DEV_RAM_COUNT;
+
+/*
* Each block ramdisk device has a radix_tree brd_pages of pages that stores
* the pages containing the block device's contents. A brd page's ->index is
* its offset in PAGE_SIZE units. This is similar to, but in no way connected
@@ -44,6 +51,12 @@ struct brd_device {
struct list_headbrd_list;
/*
+* Thin provisioning support
+*/
+ unsigned intdisk_events;
+ unsigned intpending_events;
+
+ /*
* Backing store of pages and lock to protect it. This is the contents
* of the block device.
*/
@@ -91,11 +104,23 @@ static struct page *brd_insert_page(struct brd_device
*brd, sector_t sector)
pgoff_t idx;
struct page *page;
gfp_t gfp_flags;
+ unsigned int rd_max_pages = rd_mem_size >> (PAGE_CACHE_SHIFT - 10);
+ unsigned int rd_lowat_pages = rd_lowat_thresh >> (PAGE_CACHE_SHIFT -
10);
page = brd_lookup_page(brd, sector);
if (page)
return page;
+ if (rd_total_pages >= rd_max_pages)
+ return NULL;
+
+ if (rd_total_pages >= rd_lowat_pages &&
+ !(brd->disk_events & DISK_EVENT_LOWAT)) {
+ brd->pending_events |= DISK_EVENT_LOWAT;
+ brd->disk_events |= DISK_EVENT_LOWAT;
+ disk_clear_events(brd->brd_disk, DISK_EVENT_LOWAT);
+ }
+
/*
* Must use NOIO because we don't want to recurse back into the
* block or filesystem layers from page reclaim.
@@ -127,6 +152,7 @@ static struct page *brd_insert_page(struct brd_device *brd,
sector_t sector)
BUG_ON(!page);
BUG_ON(page->index != idx);
}
+ rd_total_pages++;
spin_unlock(&brd->brd_lock);
radix_tree_preload_end();
@@ -138,10 +164,16 @@ static void brd_free_page(struct brd_device *brd,
sector_t sector)
{
struct page *page;
pgoff_t idx;
+ unsigned int rd_lowat_pages = rd_lowat_thresh >> (PAGE_CACHE_SHIFT -
10);
spin_lock(&brd->brd_lock);
idx = sector >> PAGE_SECTORS_SHIFT;
page = radix_tree_delete(&brd->brd_pages, idx);
+ rd_total_pages--;
+ if (rd_total_pages < rd_lowat_pages) {
+ brd->disk_events &= ~DISK_EVENT_LOWAT;
+ brd->pending_events &= ~DISK_EVENT_LOWAT;
+ }
spin_unlock(&brd->brd_lock);
if (page)
__free_page(page);
@@ -434,11 +466,22 @@ static int brd_ioctl(struct block_device *bdev, fmode_t
mode,
return error;
}
+static unsigned int brd_check_events(struct gendisk *disk, unsigned int mask)
+{
+ struct brd_device *brd = disk->private_data;
+ unsigned int pending_events = brd->pending_events & mask;
+
+ brd->pending_events &= ~mask;
+
+ return pending_events;
+}
+
static const struct block_device_operations brd_fops = {
.owner =THIS_MODULE,
.rw_page = brd_rw_page,
.ioctl =brd_ioctl,
.direct_access =brd_direct_access,
+ .check_events = brd_check_events,
};
/*
@@ -456,6 +499,12 @@ static int max_part = 1;
module_param(max_part, int, S_IRUGO);
MODULE_PARM_DESC(max_part, "Num Minors to reserve between devices");
+module_param(rd_mem_size, int, S_IRUGO);
+MODULE_PARM_DESC(rd_mem_size, "Maximal memory size in kbytes to allocate");
+
+module_param(rd_lowat_thresh, int, S_IRUGO);
+MODULE_PARM_DESC(rd_lowat_thresh, "Low water mark in kbytes for memory
allocation");
+
MODULE_LICENSE("GPL");
MODULE_ALIAS_BLOCKDEV_MAJOR(RAMDISK_MAJOR);
MODULE_ALIAS("rd");
@@ -519,6 +568,8 @@ static struct brd_device *brd_alloc(int i)
disk->private_data = brd;
disk->queue = brd->brd_queue;
disk->flags = GENHD_FL_EXT_DEVT;
+ disk->events= DISK_EVENT_LOWAT;
+ disk->async_events = DISK_EVENT_LOWAT;
sprintf(disk->disk_name, "ram%d", i);
set_capacity(disk, rd_size * 2);
@@ -607,6 +658,8 @@ static int __init brd_init(void)
if (register_blkdev(RAMDISK_MAJOR, "ramdisk"))
return -EIO;
+ rd_total_pages = 0;
+
i
[PATCH 3/4] dm-thin: enable low water mark disk event
Enable sending of a 'low water mark' disk event and add
supporting infrastructure to the DM core.
Signed-off-by: Hannes Reinecke
---
drivers/md/dm-thin.c | 2 ++
drivers/md/dm.c | 27 +++
drivers/md/dm.h | 3 ++-
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index 72d91f4..c191839 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -1345,6 +1345,7 @@ static void check_low_water_mark(struct pool *pool,
dm_block_t free_blocks)
spin_lock_irqsave(&pool->lock, flags);
pool->low_water_triggered = true;
spin_unlock_irqrestore(&pool->lock, flags);
+ dm_set_disk_event(pool->pool_md, DISK_EVENT_LOWAT);
dm_table_event(pool->ti->table);
}
}
@@ -4058,6 +4059,7 @@ static int thin_ctr(struct dm_target *ti, unsigned argc,
char **argv)
goto bad;
}
atomic_set(&tc->refcount, 1);
+ dm_enable_disk_event(pool_md, DISK_EVENT_LOWAT);
init_completion(&tc->can_destroy);
list_add_tail_rcu(&tc->list, &tc->pool->active_thins);
spin_unlock_irqrestore(&tc->pool->lock, flags);
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 5df4048..8d22c40 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -198,6 +198,7 @@ struct mapped_device {
wait_queue_head_t eventq;
atomic_t uevent_seq;
struct list_head uevent_list;
+ unsigned int disk_events;
spinlock_t uevent_lock; /* Protect access to uevent_list */
/*
@@ -556,6 +557,16 @@ static int dm_blk_getgeo(struct block_device *bdev, struct
hd_geometry *geo)
return dm_get_geometry(md, geo);
}
+static unsigned int dm_check_events(struct gendisk *disk, unsigned int mask)
+{
+ struct mapped_device *md = disk->private_data;
+ unsigned int pending = md->disk_events & mask;
+
+ md->disk_events &= ~mask;
+
+ return pending;
+}
+
static int dm_get_live_table_for_ioctl(struct mapped_device *md,
struct dm_target **tgt, struct block_device **bdev,
fmode_t *mode, int *srcu_idx)
@@ -2457,6 +2468,8 @@ static void event_callback(void *context)
dm_send_uevents(&uevents, &disk_to_dev(md->disk)->kobj);
+ disk_clear_events(md->disk, md->disk_events);
+
atomic_inc(&md->event_nr);
wake_up(&md->eventq);
}
@@ -3423,6 +3436,19 @@ void dm_uevent_add(struct mapped_device *md, struct
list_head *elist)
spin_unlock_irqrestore(&md->uevent_lock, flags);
}
+void dm_set_disk_event(struct mapped_device *md, unsigned int evt)
+{
+ md->disk_events |= evt;
+}
+EXPORT_SYMBOL_GPL(dm_set_disk_event);
+
+void dm_enable_disk_event(struct mapped_device *md, unsigned int evt)
+{
+ md->disk->events |= evt;
+ md->disk->async_events |= evt;
+}
+EXPORT_SYMBOL_GPL(dm_enable_disk_event);
+
/*
* The gendisk is only valid as long as you have a reference
* count on 'md'.
@@ -3678,6 +3704,7 @@ static const struct block_device_operations dm_blk_dops =
{
.ioctl = dm_blk_ioctl,
.getgeo = dm_blk_getgeo,
.pr_ops = &dm_pr_ops,
+ .check_events = dm_check_events,
.owner = THIS_MODULE
};
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index 7edcf97..fa3dc10 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -83,7 +83,8 @@ void dm_unlock_md_type(struct mapped_device *md);
void dm_set_md_type(struct mapped_device *md, unsigned type);
unsigned dm_get_md_type(struct mapped_device *md);
struct target_type *dm_get_immutable_target_type(struct mapped_device *md);
-
+void dm_set_disk_event(struct mapped_device *md, unsigned int evt);
+void dm_enable_disk_event(struct mapped_device *md, unsigned int evt);
int dm_setup_md_queue(struct mapped_device *md);
/*
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] scsi,block: enable disk event forwarding
Some SCSI events relate to block events (eg media change), so this
patch implements a forwarding mechanism for SCSI events to the
corresponding block event.
It redefines the currently unused 'supported_events' bitmap to
signal which SCSI events should be forwarded to block events.
Signed-off-by: Hannes Reinecke
---
block/genhd.c | 1 +
drivers/scsi/scsi_lib.c| 15 +--
drivers/scsi/sd.c | 26 ++
include/scsi/scsi_driver.h | 2 ++
4 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 9f42526..229c760 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1636,6 +1636,7 @@ unsigned int disk_clear_events(struct gendisk *disk,
unsigned int mask)
return pending;
}
+EXPORT_SYMBOL_GPL(disk_clear_events);
/*
* Separate this part out so that a different pointer for clearing_ptr can be
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index d46193a..6532c32 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2691,9 +2691,14 @@ EXPORT_SYMBOL(scsi_device_set_state);
*/
static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt)
{
+ struct device *dev = &sdev->sdev_gendev;
+ struct scsi_driver *sdrv = to_scsi_driver(dev->driver);
int idx = 0;
char *envp[3];
+ if (sdrv->ua_event && test_bit(evt->evt_type, sdev->supported_events))
+ sdrv->ua_event(sdev, evt->evt_type);
+
switch (evt->evt_type) {
case SDEV_EVT_MEDIA_CHANGE:
envp[idx++] = "SDEV_MEDIA_CHANGE=1";
@@ -2778,16 +2783,6 @@ void sdev_evt_send(struct scsi_device *sdev, struct
scsi_event *evt)
{
unsigned long flags;
-#if 0
- /* FIXME: currently this check eliminates all media change events
-* for polled devices. Need to update to discriminate between AN
-* and polled events */
- if (!test_bit(evt->evt_type, sdev->supported_events)) {
- kfree(evt);
- return;
- }
-#endif
-
spin_lock_irqsave(&sdev->list_lock, flags);
list_add_tail(&evt->node, &sdev->event_list);
schedule_work(&sdev->event_work);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index d749da7..b001c139 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -114,6 +114,7 @@ static int sd_init_command(struct scsi_cmnd *SCpnt);
static void sd_uninit_command(struct scsi_cmnd *SCpnt);
static int sd_done(struct scsi_cmnd *);
static int sd_eh_action(struct scsi_cmnd *, int);
+static void sd_ua_event(struct scsi_device *, enum scsi_device_event);
static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer);
static void scsi_disk_release(struct device *cdev);
static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *);
@@ -525,6 +526,7 @@ static struct scsi_driver sd_template = {
.uninit_command = sd_uninit_command,
.done = sd_done,
.eh_action = sd_eh_action,
+ .ua_event = sd_ua_event,
};
/*
@@ -1415,6 +1417,14 @@ static unsigned int sd_check_events(struct gendisk
*disk, unsigned int clearing)
goto out;
}
+ if (sdp->changed) {
+ /*
+* Media change AN
+*/
+ sdp->changed = 0;
+ return DISK_EVENT_MEDIA_CHANGE;
+ }
+
/*
* Using TEST_UNIT_READY enables differentiation between drive with
* no cartridge loaded - NOT READY, drive with changed cartridge -
@@ -1706,6 +1716,22 @@ static int sd_eh_action(struct scsi_cmnd *scmd, int
eh_disp)
return eh_disp;
}
+/**
+ * sd_ua_event - unit attention event callback
+ * @scmd: sd-issued command which triggered the UA
+ * @evt_type: Triggered event type
+ *
+ **/
+static void sd_ua_event(struct scsi_device *sdev, enum scsi_device_event evt)
+{
+ struct scsi_disk *sdkp = dev_get_drvdata(&sdev->sdev_gendev);
+
+ if (evt == SDEV_EVT_MEDIA_CHANGE) {
+ sdev->changed = 1;
+ disk_clear_events(sdkp->disk, DISK_EVENT_MEDIA_CHANGE);
+ }
+}
+
static unsigned int sd_completed_bytes(struct scsi_cmnd *scmd)
{
u64 start_lba = blk_rq_pos(scmd->request);
diff --git a/include/scsi/scsi_driver.h b/include/scsi/scsi_driver.h
index 891a658..1d1002a 100644
--- a/include/scsi/scsi_driver.h
+++ b/include/scsi/scsi_driver.h
@@ -7,6 +7,7 @@ struct module;
struct request;
struct scsi_cmnd;
struct scsi_device;
+enum scsi_device_event;
struct scsi_driver {
struct device_drivergendrv;
@@ -16,6 +17,7 @@ struct scsi_driver {
void (*uninit_command)(struct scsi_cmnd *);
int (*done)(struct scsi_cmnd *);
int (*eh_action)(struct scsi_cmnd *, int);
+ void (*ua_event)(struct scsi_device *, enum scsi_device_event evt);
};
#define to_scsi_driver(drv) \
cont
[RFC PATCH 0/4] Low water mark disk events
Hi all, here is a patchset to implement 'low water mark' disk events. This event corresponds to a TP Soft Threshold Reached UA for SCSI or a 'low watermark' event for dm-thin. It utilises the existing 'disk event' infrastructure from the blocklayer to send out the events via udev. And it also cleans up the ambiguous MEDIA_CHANGE event handling from libata, where AN (asynchronous notification) events would be signalled via SCSI events, and polled MEDIA_CHANGE events would be signalled via disk events. And I've added thin provisioning support to brd, too, to have a simple testbed for the new low water mark disk event. As usual, comments and reviews are welcome. Hannes Reinecke (4): scsi,block: enable disk event forwarding block,scsi: Low water mark disk event dm-thin: enable low water mark disk event brd: thin provisioning support block/genhd.c | 3 +++ drivers/block/brd.c| 53 ++ drivers/md/dm-thin.c | 2 ++ drivers/md/dm.c| 26 +++ drivers/md/dm.h| 3 ++- drivers/scsi/scsi_lib.c| 25 +- drivers/scsi/sd.c | 44 ++ drivers/scsi/sd.h | 1 + include/linux/genhd.h | 1 + include/scsi/scsi_driver.h | 2 ++ 10 files changed, 144 insertions(+), 16 deletions(-) -- 1.8.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] block,scsi: Low water mark disk event
Add a disk event for a 'low water mark' condition, signalling when
a device is about to run out of space. This event is mapped to a
Thin Provisioning Soft Threshold Reached UA.
Signed-off-by: Hannes Reinecke
---
block/genhd.c | 2 ++
drivers/scsi/scsi_lib.c | 10 +-
drivers/scsi/sd.c | 19 +++
drivers/scsi/sd.h | 1 +
include/linux/genhd.h | 1 +
5 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 229c760..48334e6 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1437,11 +1437,13 @@ struct disk_events {
static const char *disk_events_strs[] = {
[ilog2(DISK_EVENT_MEDIA_CHANGE)]= "media_change",
[ilog2(DISK_EVENT_EJECT_REQUEST)] = "eject_request",
+ [ilog2(DISK_EVENT_LOWAT)] = "low_water_mark",
};
static char *disk_uevents[] = {
[ilog2(DISK_EVENT_MEDIA_CHANGE)]= "DISK_MEDIA_CHANGE=1",
[ilog2(DISK_EVENT_EJECT_REQUEST)] = "DISK_EJECT_REQUEST=1",
+ [ilog2(DISK_EVENT_LOWAT)] = "DISK_LOW_WATER_MARK=1",
};
/* list of all disk_events */
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6532c32..e8955da 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2683,7 +2683,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum
scsi_device_state state)
EXPORT_SYMBOL(scsi_device_set_state);
/**
- * sdev_evt_emit - emit a single SCSI device uevent
+ * sdev_evt_emit - emit a single SCSI device uevent
* @sdev: associated SCSI device
* @evt: event to emit
*
@@ -2711,7 +2711,7 @@ static void scsi_evt_emit(struct scsi_device *sdev,
struct scsi_event *evt)
envp[idx++] = "SDEV_UA=CAPACITY_DATA_HAS_CHANGED";
break;
case SDEV_EVT_SOFT_THRESHOLD_REACHED_REPORTED:
- envp[idx++] = "SDEV_UA=THIN_PROVISIONING_SOFT_THRESHOLD_REACHED";
+ envp[idx++] =
"SDEV_UA=THIN_PROVISIONING_SOFT_THRESHOLD_REACHED";
break;
case SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED:
envp[idx++] = "SDEV_UA=MODE_PARAMETERS_CHANGED";
@@ -2733,7 +2733,7 @@ static void scsi_evt_emit(struct scsi_device *sdev,
struct scsi_event *evt)
}
/**
- * sdev_evt_thread - send a uevent for each scsi event
+ * sdev_evt_thread - send a uevent for each scsi event
* @work: work struct for scsi_device
*
* Dispatch queued events to their associated scsi_device kobjects
@@ -2773,7 +2773,7 @@ void scsi_evt_thread(struct work_struct *work)
}
/**
- * sdev_evt_send - send asserted event to uevent thread
+ * sdev_evt_send - send asserted event to uevent thread
* @sdev: scsi_device event occurred on
* @evt: event to send
*
@@ -2791,7 +2791,7 @@ void sdev_evt_send(struct scsi_device *sdev, struct
scsi_event *evt)
EXPORT_SYMBOL_GPL(sdev_evt_send);
/**
- * sdev_evt_alloc - allocate a new scsi event
+ * sdev_evt_alloc - allocate a new scsi event
* @evt_type: type of event to allocate
* @gfpflags: GFP flags for allocation
*
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b001c139..34de425 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1425,6 +1425,16 @@ static unsigned int sd_check_events(struct gendisk
*disk, unsigned int clearing)
return DISK_EVENT_MEDIA_CHANGE;
}
+ if (sdkp->tp_lowat) {
+ /*
+* Thin Provisioning Low Watermark reached;
+* don't send TEST_UNIT_READY but rather return
+* immediately.
+*/
+ sdkp->tp_lowat = false;
+ return DISK_EVENT_LOWAT;
+ }
+
/*
* Using TEST_UNIT_READY enables differentiation between drive with
* no cartridge loaded - NOT READY, drive with changed cartridge -
@@ -1729,6 +1739,9 @@ static void sd_ua_event(struct scsi_device *sdev, enum
scsi_device_event evt)
if (evt == SDEV_EVT_MEDIA_CHANGE) {
sdev->changed = 1;
disk_clear_events(sdkp->disk, DISK_EVENT_MEDIA_CHANGE);
+ } else if (evt == SDEV_EVT_SOFT_THRESHOLD_REACHED_REPORTED) {
+ sdkp->tp_lowat = true;
+ disk_clear_events(sdkp->disk, DISK_EVENT_LOWAT);
}
}
@@ -3044,6 +3057,12 @@ static void sd_probe_async(void *data, async_cookie_t
cookie)
gd->flags |= GENHD_FL_REMOVABLE;
gd->events |= DISK_EVENT_MEDIA_CHANGE;
}
+ if (sdkp->lbpme) {
+ gd->events |= DISK_EVENT_LOWAT;
+ gd->async_events |= DISK_EVENT_LOWAT;
+ set_bit(SDEV_EVT_SOFT_THRESHOLD_REACHED_REPORTED,
+ sdp->supported_events);
+ }
blk_pm_runtime_init(sdp->request_queue, dev);
add_disk(gd);
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 5f2a84a..b22b8f0 100644
--- a/drivers/scsi/sd.h
+++
PROBLEM: cdrom.c : cdrom_get_last_written() yields up to 3 different values
cdrom.c : cdrom_get_last_written() yields up to 3 different values
--
Full description of the problem:
Depending on the capabilities of the optical drive involved, the function
cdrom_get_last_written() can yield up to 3 different results with the same CD.
The function has 4 callers with at least 2 different expectations.
This is not one of the known drive quirks but a matter of slightly different
semantics of the different SCSI reply fields used.
With a TAO CD there are three possible outcomes:
payload size - 1, payload size, payload size + 2
With a SAO CD there are 2 different results:
payload size - 1, payload size
--
The 3 result variants are:
1) If the drive does not sufficiently react on GET DISC INFORMATION
or READ TRACK INFORMATION, then the function falls back to label
"use_toc:" which uses READ TOC/PMA/ATIP to inquire the start of the
Lead-out:
toc.cdte_addr.lba
This is the payload size of the TAO CD plus 2 run-out blocks.
With a SAO CD it is the payload size.
None of my 7 drives would go this path.
2) If the reply to READ TRACK INFORMATION of the last track does not have
the LRA_V bit set (i.e. the Last Recorded Address field is not valid),
then the function falls back to the reply fields Logical Track Start Address
and Logical Track Size:
be32_to_cpu(ti.track_start) + be32_to_cpu(ti.track_size)
This is the payload size of the TAO CD on 6 of my 7 drives.
(One of them has LRA_V set, though, and thus takes path 3.
My oldest DVD-ROM returns payload + run-out, which is equal to the
Lead-out start reported by READ TOC/PMA/ATIP. For this lie, the kernel
is not to blame.)
With SAO this is the payload size.
3) If the READ TRACK INFORMATION reply has LRA_V set, then the function
uses the value of the reply field Last Recorded Address:
be32_to_cpu(ti.last_rec_address)
This reply field is promised by MMC-5 only for [HD]DVD-R[W] and BD-R
media. (See MMC-5, 6.27.3.18 "Last Recorded Address".)
It is supposed to be the address of the last payload block.
I.e. payload size - 1. SAO and TAO alike.
Only one of my drives has the LRA_V bit set with CD-RW.
--
The 4 callers are:
a) drivers/scsi/sr.c : get_sectorsize()
Obviously expects payload size.
(This is the one i am actually interested in.)
b) drivers/ide/ide-cd.c : ide_cd_read_toc()
Obviously expects payload size.
c) drivers/cdrom/cdrom.c : cdrom_get_next_writable()
Falls back to cdrom_get_last_written() and adds 7 to the value.
(I am curious about the reasoning for 7 and whether it shall
be added to payload size or to payload size - 1.)
d) drivers/cdrom/cdrom.c : mmc_ioctl_cdrom_last_written()
Implements ioctl(CDROM_LAST_WRITTEN) which promises "get last block
written on disc" in include/uapi/linux/cdrom.h.
So this expects payload size - 1.
No caller seems to expect payload size + 2.
--
Source inspected:
git://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git
(I came to the problem when i examined the possibility to let the
last block address from READ TRACK INFORMATION override the reply
of READ CAPACITY if it gets deceived by TAO tracks.
It would fix the TAO CD end problems for 4 of my 5 lying drives.)
--
Hardware behavior:
Example READ TRACK INFORMATION transactions of my drives (libburn log)
with a TAO CD with 51623 payload blocks:
'TSSTcorp' 'DVD-ROM SH-D162C' reports 0xc9a9 = 51625:
READ TRACK INFORMATION
52 01 00 00 00 01 00 00 22 00
From drive: 34b
00 2e 01 01 00 04 01 00 00 00 00 00 00 00 c9 a9 00 00 00 00
00 00 00 00 00 00 c9 a9 00 a2 00 00 00 00
'TSSTcorp' 'CDDVDW SH-S223B' reports 0xc9a7 = 51623:
READ TRACK INFORMATION
52 01 00 00 00 01 00 00 22 00
From drive: 34b
00 1a 01 01 00 04 81 00 00 00 00 00 ff ff ff ff 00 00 00 00
00 00 00 00 00 00 c9 a7 00 a2 00 00 00 00
'HL-DT-ST' 'BDDVDRW GGC-H20L'
'HL-DT-ST' 'DVDRAM GH24NSC0'
'HL-DT-ST' 'BD-RE GGW-H20L'
'HL-DT-ST' 'BD-RE BH16NS40' report 0xc9a7 = 51623:
READ TRACK INFORMATION
52 01 00 00 00 01 00 00 22 00
From drive: 34b
00 22 01 01 00 04 01 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 c9 a7 00 00 00 00 00 00
'Optiarc ' 'BD RW BD-5300S' reports LRA_V == 1 and
Last Recorded Address 0xc9a6 = 51622:
READ TRACK INFORMATION
52 01 00 00 00 01 00 00 22 00
From drive: 34b
00 2e 01 01 00 04 81 02 00 00 00 00 ff ff ff ff 00 00 00 00
00 00 00 00 00 00 c9 a7 00 00 c9 a6 00 00
All look the same with READ TOC/PMA/ATIP format 2 ("raw"):
READ TOC/PMA/ATIP
43 02 02 00 00 00 00 00 30 00
From drive: 48b
00 2e 01 01 01 14 00 a0 00 00 00 00 01 00 00 01 14 00 a1 00
00 00 00 01 00 00 01 14 00 a2 00 00 00 00 0b 1e 19 01 14 00
01 00 00 00 00 00 02 00
which means:
Lead-out st
Re: [PATCH resend 2/2] mpt3sas: Remove unnecessary synchronize_irq() before free_irq()
On Wed, Mar 9, 2016 at 7:22 AM, Martin K. Petersen wrote: >> "Lars-Peter" == Lars-Peter Clausen writes: > > Lars-Peter> Calling synchronize_irq() right before free_irq() is quite > Lars-Peter> useless. On one hand the IRQ can easily fire again before > Lars-Peter> free_irq() is entered, on the other hand free_irq() itself > Lars-Peter> calls synchronize_irq() internally (in a race condition free > Lars-Peter> way), before any state associated with the IRQ is freed. > > Broadcom folks, please review. Martin, Please consider this patch as Ack-by: Sreekanth Reddy Thanks, Sreekanth > > -- > Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5] scsi: ufs: add ioctl interface for query request
This patch exposes the ioctl interface for UFS driver via SCSI device
ioctl interface. As of now UFS driver would provide the ioctl for query
interface to connected UFS device.
Reviewed-by: Subhash Jadavani
Signed-off-by: Dolev Raviv
Signed-off-by: Gilad Broner
Signed-off-by: Yaniv Gardi
---
drivers/scsi/ufs/ufs.h| 53 +++
drivers/scsi/ufs/ufshcd.c | 208 +-
include/uapi/scsi/Kbuild | 1 +
include/uapi/scsi/ufs/Kbuild | 3 +
include/uapi/scsi/ufs/ioctl.h | 58
include/uapi/scsi/ufs/ufs.h | 67 ++
6 files changed, 347 insertions(+), 43 deletions(-)
create mode 100644 include/uapi/scsi/ufs/Kbuild
create mode 100644 include/uapi/scsi/ufs/ioctl.h
create mode 100644 include/uapi/scsi/ufs/ufs.h
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index b291fa6..d39410f 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -38,6 +38,7 @@
#include
#include
+#include
#define MAX_CDB_SIZE 16
#define GENERAL_UPIU_REQUEST_SIZE 32
@@ -72,6 +73,16 @@ enum {
UFS_UPIU_RPMB_WLUN = 0xC4,
};
+/**
+ * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
+ * @lun: LU number to check
+ * @return: true if the lun has a matching unit descriptor, false otherwise
+ */
+static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
+{
+ return lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN);
+}
+
/*
* UFS Protocol Information Unit related definitions
*/
@@ -127,35 +138,6 @@ enum {
UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81,
};
-/* Flag idn for Query Requests*/
-enum flag_idn {
- QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
- QUERY_FLAG_IDN_PWR_ON_WPE = 0x03,
- QUERY_FLAG_IDN_BKOPS_EN = 0x04,
-};
-
-/* Attribute idn for Query requests */
-enum attr_idn {
- QUERY_ATTR_IDN_ACTIVE_ICC_LVL = 0x03,
- QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
- QUERY_ATTR_IDN_EE_CONTROL = 0x0D,
- QUERY_ATTR_IDN_EE_STATUS= 0x0E,
-};
-
-/* Descriptor idn for Query requests */
-enum desc_idn {
- QUERY_DESC_IDN_DEVICE = 0x0,
- QUERY_DESC_IDN_CONFIGURAION = 0x1,
- QUERY_DESC_IDN_UNIT = 0x2,
- QUERY_DESC_IDN_RFU_0= 0x3,
- QUERY_DESC_IDN_INTERCONNECT = 0x4,
- QUERY_DESC_IDN_STRING = 0x5,
- QUERY_DESC_IDN_RFU_1= 0x6,
- QUERY_DESC_IDN_GEOMETRY = 0x7,
- QUERY_DESC_IDN_POWER= 0x8,
- QUERY_DESC_IDN_MAX,
-};
-
enum desc_header_offset {
QUERY_DESC_LENGTH_OFFSET= 0x00,
QUERY_DESC_DESC_TYPE_OFFSET = 0x01,
@@ -279,19 +261,6 @@ enum bkops_status {
BKOPS_STATUS_MAX = BKOPS_STATUS_CRITICAL,
};
-/* UTP QUERY Transaction Specific Fields OpCode */
-enum query_opcode {
- UPIU_QUERY_OPCODE_NOP = 0x0,
- UPIU_QUERY_OPCODE_READ_DESC = 0x1,
- UPIU_QUERY_OPCODE_WRITE_DESC= 0x2,
- UPIU_QUERY_OPCODE_READ_ATTR = 0x3,
- UPIU_QUERY_OPCODE_WRITE_ATTR= 0x4,
- UPIU_QUERY_OPCODE_READ_FLAG = 0x5,
- UPIU_QUERY_OPCODE_SET_FLAG = 0x6,
- UPIU_QUERY_OPCODE_CLEAR_FLAG= 0x7,
- UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
-};
-
/* Query response result code */
enum {
QUERY_RESULT_SUCCESS= 0x00,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index da882cf..42f1e7f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -38,6 +38,7 @@
*/
#include
+#include
#include
#include
#include
@@ -2178,7 +2179,7 @@ static inline int ufshcd_read_unit_desc_param(struct
ufs_hba *hba,
* Unit descriptors are only available for general purpose LUs (LUN id
* from 0 to 7) and RPMB Well known LU.
*/
- if (lun != UFS_UPIU_RPMB_WLUN && (lun >= UFS_UPIU_MAX_GENERAL_LUN))
+ if (!ufs_is_valid_unit_desc_lun(lun))
return -EOPNOTSUPP;
return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
@@ -5001,6 +5002,207 @@ out:
}
/**
+ * ufshcd_query_ioctl - perform user read queries
+ * @hba: per-adapter instance
+ * @lun: used for lun specific queries
+ * @buffer: user space buffer for reading and submitting query data and params
+ * @return: 0 for success negative error code otherwise
+ *
+ * Expected/Submitted buffer structure is struct ufs_ioctl_query_data.
+ * It will read the opcode, idn and buf_length parameters, and, put the
+ * response in the buffer field while updating the used size in buf_length.
+ */
+static int ufshcd_query_ioctl(struct ufs_hba *hba, u8 lun, void __user *buffer)
+{
+ struct ufs_ioctl_query_data ioctl_data;
+ int err = 0;
+ int length = 0;
+ void *data_ptr;
+ bool flag;
+ u32 att;
+ u8 index;
+ u8 *desc = NULL;
+
+ /* extract params f
[PATCH v6] scsi: ufs: add ioctl interface for query request
This patch exposes the ioctl interface for UFS driver via SCSI device
ioctl interface. As of now UFS driver would provide the ioctl for query
interface to connected UFS device.
Reviewed-by: Arnd Bergmann
Reviewed-by: Subhash Jadavani
Signed-off-by: Dolev Raviv
Signed-off-by: Noa Rubens
Signed-off-by: Raviv Shvili
Signed-off-by: Gilad Broner
Signed-off-by: Yaniv Gardi
---
drivers/scsi/ufs/ufs.h| 53 +++
drivers/scsi/ufs/ufshcd.c | 208 +-
include/uapi/scsi/Kbuild | 1 +
include/uapi/scsi/ufs/Kbuild | 3 +
include/uapi/scsi/ufs/ioctl.h | 58
include/uapi/scsi/ufs/ufs.h | 67 ++
6 files changed, 347 insertions(+), 43 deletions(-)
create mode 100644 include/uapi/scsi/ufs/Kbuild
create mode 100644 include/uapi/scsi/ufs/ioctl.h
create mode 100644 include/uapi/scsi/ufs/ufs.h
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index b291fa6..d39410f 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -38,6 +38,7 @@
#include
#include
+#include
#define MAX_CDB_SIZE 16
#define GENERAL_UPIU_REQUEST_SIZE 32
@@ -72,6 +73,16 @@ enum {
UFS_UPIU_RPMB_WLUN = 0xC4,
};
+/**
+ * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
+ * @lun: LU number to check
+ * @return: true if the lun has a matching unit descriptor, false otherwise
+ */
+static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
+{
+ return lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN);
+}
+
/*
* UFS Protocol Information Unit related definitions
*/
@@ -127,35 +138,6 @@ enum {
UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81,
};
-/* Flag idn for Query Requests*/
-enum flag_idn {
- QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
- QUERY_FLAG_IDN_PWR_ON_WPE = 0x03,
- QUERY_FLAG_IDN_BKOPS_EN = 0x04,
-};
-
-/* Attribute idn for Query requests */
-enum attr_idn {
- QUERY_ATTR_IDN_ACTIVE_ICC_LVL = 0x03,
- QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
- QUERY_ATTR_IDN_EE_CONTROL = 0x0D,
- QUERY_ATTR_IDN_EE_STATUS= 0x0E,
-};
-
-/* Descriptor idn for Query requests */
-enum desc_idn {
- QUERY_DESC_IDN_DEVICE = 0x0,
- QUERY_DESC_IDN_CONFIGURAION = 0x1,
- QUERY_DESC_IDN_UNIT = 0x2,
- QUERY_DESC_IDN_RFU_0= 0x3,
- QUERY_DESC_IDN_INTERCONNECT = 0x4,
- QUERY_DESC_IDN_STRING = 0x5,
- QUERY_DESC_IDN_RFU_1= 0x6,
- QUERY_DESC_IDN_GEOMETRY = 0x7,
- QUERY_DESC_IDN_POWER= 0x8,
- QUERY_DESC_IDN_MAX,
-};
-
enum desc_header_offset {
QUERY_DESC_LENGTH_OFFSET= 0x00,
QUERY_DESC_DESC_TYPE_OFFSET = 0x01,
@@ -279,19 +261,6 @@ enum bkops_status {
BKOPS_STATUS_MAX = BKOPS_STATUS_CRITICAL,
};
-/* UTP QUERY Transaction Specific Fields OpCode */
-enum query_opcode {
- UPIU_QUERY_OPCODE_NOP = 0x0,
- UPIU_QUERY_OPCODE_READ_DESC = 0x1,
- UPIU_QUERY_OPCODE_WRITE_DESC= 0x2,
- UPIU_QUERY_OPCODE_READ_ATTR = 0x3,
- UPIU_QUERY_OPCODE_WRITE_ATTR= 0x4,
- UPIU_QUERY_OPCODE_READ_FLAG = 0x5,
- UPIU_QUERY_OPCODE_SET_FLAG = 0x6,
- UPIU_QUERY_OPCODE_CLEAR_FLAG= 0x7,
- UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
-};
-
/* Query response result code */
enum {
QUERY_RESULT_SUCCESS= 0x00,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index da882cf..42f1e7f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -38,6 +38,7 @@
*/
#include
+#include
#include
#include
#include
@@ -2178,7 +2179,7 @@ static inline int ufshcd_read_unit_desc_param(struct
ufs_hba *hba,
* Unit descriptors are only available for general purpose LUs (LUN id
* from 0 to 7) and RPMB Well known LU.
*/
- if (lun != UFS_UPIU_RPMB_WLUN && (lun >= UFS_UPIU_MAX_GENERAL_LUN))
+ if (!ufs_is_valid_unit_desc_lun(lun))
return -EOPNOTSUPP;
return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
@@ -5001,6 +5002,207 @@ out:
}
/**
+ * ufshcd_query_ioctl - perform user read queries
+ * @hba: per-adapter instance
+ * @lun: used for lun specific queries
+ * @buffer: user space buffer for reading and submitting query data and params
+ * @return: 0 for success negative error code otherwise
+ *
+ * Expected/Submitted buffer structure is struct ufs_ioctl_query_data.
+ * It will read the opcode, idn and buf_length parameters, and, put the
+ * response in the buffer field while updating the used size in buf_length.
+ */
+static int ufshcd_query_ioctl(struct ufs_hba *hba, u8 lun, void __user *buffer)
+{
+ struct ufs_ioctl_query_data ioctl_data;
+ int err = 0;
+ int length = 0;
+ void *data_ptr;
+ bool flag;
+
[PATCH v7] scsi: ufs: add ioctl interface for query request
This patch exposes the ioctl interface for UFS driver via SCSI device
ioctl interface. As of now UFS driver would provide the ioctl for query
interface to connected UFS device.
Reviewed-by: Subhash Jadavani
Signed-off-by: Dolev Raviv
Signed-off-by: Gilad Broner
Signed-off-by: Yaniv Gardi
---
drivers/scsi/ufs/ufs.h| 53 +++
drivers/scsi/ufs/ufshcd.c | 208 +-
include/uapi/scsi/Kbuild | 1 +
include/uapi/scsi/ufs/Kbuild | 3 +
include/uapi/scsi/ufs/ioctl.h | 58
include/uapi/scsi/ufs/ufs.h | 67 ++
6 files changed, 347 insertions(+), 43 deletions(-)
create mode 100644 include/uapi/scsi/ufs/Kbuild
create mode 100644 include/uapi/scsi/ufs/ioctl.h
create mode 100644 include/uapi/scsi/ufs/ufs.h
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index b291fa6..d39410f 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -38,6 +38,7 @@
#include
#include
+#include
#define MAX_CDB_SIZE 16
#define GENERAL_UPIU_REQUEST_SIZE 32
@@ -72,6 +73,16 @@ enum {
UFS_UPIU_RPMB_WLUN = 0xC4,
};
+/**
+ * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
+ * @lun: LU number to check
+ * @return: true if the lun has a matching unit descriptor, false otherwise
+ */
+static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
+{
+ return lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN);
+}
+
/*
* UFS Protocol Information Unit related definitions
*/
@@ -127,35 +138,6 @@ enum {
UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81,
};
-/* Flag idn for Query Requests*/
-enum flag_idn {
- QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
- QUERY_FLAG_IDN_PWR_ON_WPE = 0x03,
- QUERY_FLAG_IDN_BKOPS_EN = 0x04,
-};
-
-/* Attribute idn for Query requests */
-enum attr_idn {
- QUERY_ATTR_IDN_ACTIVE_ICC_LVL = 0x03,
- QUERY_ATTR_IDN_BKOPS_STATUS = 0x05,
- QUERY_ATTR_IDN_EE_CONTROL = 0x0D,
- QUERY_ATTR_IDN_EE_STATUS= 0x0E,
-};
-
-/* Descriptor idn for Query requests */
-enum desc_idn {
- QUERY_DESC_IDN_DEVICE = 0x0,
- QUERY_DESC_IDN_CONFIGURAION = 0x1,
- QUERY_DESC_IDN_UNIT = 0x2,
- QUERY_DESC_IDN_RFU_0= 0x3,
- QUERY_DESC_IDN_INTERCONNECT = 0x4,
- QUERY_DESC_IDN_STRING = 0x5,
- QUERY_DESC_IDN_RFU_1= 0x6,
- QUERY_DESC_IDN_GEOMETRY = 0x7,
- QUERY_DESC_IDN_POWER= 0x8,
- QUERY_DESC_IDN_MAX,
-};
-
enum desc_header_offset {
QUERY_DESC_LENGTH_OFFSET= 0x00,
QUERY_DESC_DESC_TYPE_OFFSET = 0x01,
@@ -279,19 +261,6 @@ enum bkops_status {
BKOPS_STATUS_MAX = BKOPS_STATUS_CRITICAL,
};
-/* UTP QUERY Transaction Specific Fields OpCode */
-enum query_opcode {
- UPIU_QUERY_OPCODE_NOP = 0x0,
- UPIU_QUERY_OPCODE_READ_DESC = 0x1,
- UPIU_QUERY_OPCODE_WRITE_DESC= 0x2,
- UPIU_QUERY_OPCODE_READ_ATTR = 0x3,
- UPIU_QUERY_OPCODE_WRITE_ATTR= 0x4,
- UPIU_QUERY_OPCODE_READ_FLAG = 0x5,
- UPIU_QUERY_OPCODE_SET_FLAG = 0x6,
- UPIU_QUERY_OPCODE_CLEAR_FLAG= 0x7,
- UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
-};
-
/* Query response result code */
enum {
QUERY_RESULT_SUCCESS= 0x00,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index da882cf..65a9d39 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -38,6 +38,7 @@
*/
#include
+#include
#include
#include
#include
@@ -2178,7 +2179,7 @@ static inline int ufshcd_read_unit_desc_param(struct
ufs_hba *hba,
* Unit descriptors are only available for general purpose LUs (LUN id
* from 0 to 7) and RPMB Well known LU.
*/
- if (lun != UFS_UPIU_RPMB_WLUN && (lun >= UFS_UPIU_MAX_GENERAL_LUN))
+ if (!ufs_is_valid_unit_desc_lun(lun))
return -EOPNOTSUPP;
return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
@@ -5001,6 +5002,207 @@ out:
}
/**
+ * ufshcd_query_ioctl - perform user read queries
+ * @hba: per-adapter instance
+ * @lun: used for lun specific queries
+ * @buffer: user space buffer for reading and submitting query data and params
+ * @return: 0 for success negative error code otherwise
+ *
+ * Expected/Submitted buffer structure is struct ufs_ioctl_query_data.
+ * It will read the opcode, idn and buf_length parameters, and, put the
+ * response in the buffer field while updating the used size in buf_length.
+ */
+static int ufshcd_query_ioctl(struct ufs_hba *hba, u8 lun, void __user *buffer)
+{
+ struct ufs_ioctl_query_data ioctl_data;
+ int err = 0;
+ int length = 0;
+ void *data_ptr;
+ bool flag;
+ u32 att;
+ u8 index;
+ u8 *desc = NULL;
+
+ /* extract params f
Re: [BUG] "block: make generic_make_request handle arbitrarily sized bios" breaks boot on parisc-linux
On Wed, Mar 9, 2016 at 8:55 PM, James Bottomley
wrote:
> On Thu, 2016-02-25 at 11:38 +0800, Ming Lei wrote:
>> On Thu, Feb 25, 2016 at 7:28 AM, John David Anglin <
>> dave.ang...@bell.net> wrote:
>> > On 2016-02-24, at 4:36 PM, Helge Deller wrote:
>> >
>> > > Maybe Dave has more luck, otherwise I'll continue to try to get
>> > > some info.
>> >
>> > I tried your patch on the commit in linux-block which first failed
>> > to boot. As with Helge, the
>> > system crashed and no useful data was output on console. I then
>> > applied following patch
>> > to give some extra segments and tired again:
>> >
>> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> > index b1a2631..b421f03 100644
>> > --- a/drivers/scsi/scsi_lib.c
>> > +++ b/drivers/scsi/scsi_lib.c
>> > @@ -595,6 +595,11 @@ static int scsi_alloc_sgtable(struct
>> > scsi_data_buffer *sdb, int nents, bool mq)
>> >
>> > BUG_ON(!nents);
>> >
>> > + /* Provide extra entries in case of split. */
>> > + nents += 8;
>> > + if (nents > SCSI_MAX_SG_SEGMENTS)
>> > + nents = SCSI_MAX_SG_SEGMENTS;
>> > +
>>
>> Yeah, this is needed for sake of safety.
>>
>> > if (mq) {
>> > if (nents <= SCSI_MAX_SG_SEGMENTS) {
>> > sdb->table.nents = nents;
>> >
>> > The attached file shows the crash in first boot. The second boot
>> > was successful and various output
>> > was generated by your check code.
>>
>> From the following log(just select one simple, and looks all are
>> similar) in
>> 2nd boot, the bi_phys_segments is figured out as one by block core
>> , which is wrong because the max segment size is 64k according to
>> your investigation in the below link, but the whole req/bio is
>> 192k(4k*48).
>>
>> http://www.spinics.net/lists/linux-parisc/msg06749.html
>>
>> Looks weird, it shouldn't have happened because
>> blk_bio_segment_split()
>> does respect the max segment size limit.
>>
>> BTW, what is the scsi driver for the device?
>>
>> blk_rq_map_sg: merge bug: 3 1, extra_len 0, dma_drain 0
>> check_bvec: dump bvec for 7e53c5f0(f:2449, t:1)
>> 0: 0 4096 245852 7e2c4c40
>> 1: 0 4096 245853 7e2c4c40
>> 2: 0 4096 245854 7e2c4c40
>> 3: 0 4096 245855 7e2c4c40
>> 4: 0 4096 245856 7e2c4c40
>> 5: 0 4096 245857 7e2c4c40
>> 6: 0 4096 245858 7e2c4c40
>> 7: 0 4096 245859 7e2c4c40
>> 8: 0 4096 245860 7e2c4c40
>> 9: 0 4096 245861 7e2c4c40
>>10: 0 4096 245862 7e2c4c40
>>11: 0 4096 245863 7e2c4c40
>>12: 0 4096 245864 7e2c4c40
>>13: 0 4096 245865 7e2c4c40
>>14: 0 4096 245866 7e2c4c40
>>15: 0 4096 245867 7e2c4c40
>>16: 0 4096 245868 7e2c4c40
>>17: 0 4096 245869 7e2c4c40
>>18: 0 4096 245870 7e2c4c40
>>19: 0 4096 245871 7e2c4c40
>>20: 0 4096 245872 7e2c4c40
>>21: 0 4096 245873 7e2c4c40
>>22: 0 4096 245874 7e2c4c40
>>23: 0 4096 245875 7e2c4c40
>>24: 0 4096 245876 7e2c4c40
>>25: 0 4096 245877 7e2c4c40
>>26: 0 4096 245878 7e2c4c40
>>27: 0 4096 245879 7e2c4c40
>>28: 0 4096 245880 7e2c4c40
>>29: 0 4096 245881 7e2c4c40
>>30: 0 4096 245882 7e2c4c40
>>31: 0 4096 245883 7e2c4c40
>>32: 0 4096 245884 7e2c4c40
>>33: 0 4096 245885 7e2c4c40
>>34: 0 4096 245886 7e2c4c40
>>35: 0 4096 245887 7e2c4c40
>>36: 0 4096 245888 7e2c4c40
>>37: 0 4096 245889 7e2c4c40
>>38: 0 4096 245890 7e2c4c40
>>39: 0 4096 245891 7e2c4c40
>>40: 0 4096 245892 7e2c4c40
>>41: 0 4096 245893 7e2c4c40
>>42: 0 4096 245894 7e2c4c40
>>43: 0 4096 245895 7e2c4c40
>>44: 0 4096 245896 7e2c4c40
>>45: 0 4096 245897 7e2c4c40
>>46: 0 4096 245898 7e2c4c40
>>47: 0 4096 245899 7e2c4c40
>
>
> We've provided all the information you asked for, what's the next step
> on this, or do we have to unwind the bio splitting code with reverts
> until it starts working?
John, Helge, and I did discuss the problem for a while privately, and looks
it is related with compiler. Last time, I sent one patch which can make the
issue disappear, but the main change is just invovled with the below:
struct bio_vec {
struct page*bv_page;
-unsigned intbv_len;
+unsigned intbv_seg:8;
+
我的个人主页是
你的老朋友邀你来Q群:343257759
Re: [BUG] "block: make generic_make_request handle arbitrarily sized bios" breaks boot on parisc-linux
On 2016-03-09 9:43 AM, Ming Lei wrote:
We've provided all the information you asked for, what's the next step
>on this, or do we have to unwind the bio splitting code with reverts
>until it starts working?
John, Helge, and I did discuss the problem for a while privately, and looks
it is related with compiler. Last time, I sent one patch which can make the
issue disappear, but the main change is just invovled with the below:
struct bio_vec {
struct page*bv_page;
-unsigned intbv_len;
+unsigned intbv_seg:8;
+unsigned intbv_len:24;
unsigned intbv_offset;
};
Maybe John and Helge have some update recently?
The logic in blk_bio_segment_split() is correct, and it does respect the max
segment size limit.
Helge has found that tagging blk_bio_segment_split() with "__attribute__
((optimize("O0")))"
makes the issue disappear. The bug remains if one just adds bv_len to
the struct without the
bit fields. Maybe problem is evident from following output which I sent
to Ming and Helge
last weekend?
blk_rq_map_sg: merge bug: 3 2, extra_len 0, dma_drain 0
check_bvec: dump bvec for 7e4efdc0(f:2449, t:1)
0: 0 4096 246503 7e4a4f00(0, 94208, 1)
1: 0 4096 246504 7e4a4f00(0, 94208, 1)
2: 0 4096 246505 7e4a4f00(0, 94208, 1)
3: 0 4096 246506 7e4a4f00(0, 94208, 1)
4: 0 4096 246538 7e4a4f00(0, 94208, 2)
5: 0 4096 246539 7e4a4f00(0, 94208, 2)
6: 0 4096 246540 7e4a4f00(0, 94208, 2)
7: 0 4096 246541 7e4a4f00(0, 94208, 2)
8: 0 4096 246542 7e4a4f00(0, 94208, 2)
9: 0 4096 246543 7e4a4f00(0, 94208, 2)
10: 0 4096 246544 7e4a4f00(0, 94208, 2)
11: 0 4096 246545 7e4a4f00(0, 94208, 2)
12: 0 4096 246546 7e4a4f00(0, 94208, 2)
13: 0 4096 246547 7e4a4f00(0, 94208, 2)
14: 0 4096 246548 7e4a4f00(0, 94208, 2)
15: 0 4096 246549 7e4a4f00(0, 94208, 2)
16: 0 4096 246550 7e4a4f00(0, 94208, 2)
17: 0 4096 246551 7e4a4f00(0, 94208, 2)
18: 0 4096 246552 7e4a4f00(0, 94208, 2)
19: 0 4096 246553 7e4a4f00(0, 94208, 2)
20: 0 4096 246554 7e4a4f00(0, 94208, 2)
21: 0 4096 246555 7e4a4f00(0, 94208, 2)
22: 0 4096 246556 7e4a4f00(0, 94208, 2)
Kernel panic - not syncing: bad block merge
It seems segment 1 is too small and segment 2 too big?
The general plan is to disable inlining (maybe move
blk_bio_segment_split() to a separate
function) to try to figure out what is miscompiled.
As you say, this is probably a GCC bug. However, it's likely a
middle-end or optimization
bug in the common GCC code.
Dave
--
John David Anglin dave.ang...@bell.net
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] "block: make generic_make_request handle arbitrarily sized bios" breaks boot on parisc-linux
On Wed, Mar 9, 2016 at 11:15 PM, John David Anglin wrote:
> On 2016-03-09 9:43 AM, Ming Lei wrote:
>>>
>>> We've provided all the information you asked for, what's the next step
>>> >on this, or do we have to unwind the bio splitting code with reverts
>>> >until it starts working?
>>
>> John, Helge, and I did discuss the problem for a while privately, and
>> looks
>> it is related with compiler. Last time, I sent one patch which can make
>> the
>> issue disappear, but the main change is just invovled with the below:
>>
>> struct bio_vec {
>> struct page*bv_page;
>> -unsigned intbv_len;
>> +unsigned intbv_seg:8;
>> +unsigned intbv_len:24;
>> unsigned intbv_offset;
>> };
>>
>> Maybe John and Helge have some update recently?
>>
>> The logic in blk_bio_segment_split() is correct, and it does respect the
>> max
>> segment size limit.
>
> Helge has found that tagging blk_bio_segment_split() with "__attribute__
> ((optimize("O0")))"
> makes the issue disappear. The bug remains if one just adds bv_len to the
> struct without the
> bit fields. Maybe problem is evident from following output which I sent to
> Ming and Helge
> last weekend?
>
> blk_rq_map_sg: merge bug: 3 2, extra_len 0, dma_drain 0
> check_bvec: dump bvec for 7e4efdc0(f:2449, t:1)
> 0: 0 4096 246503 7e4a4f00(0, 94208, 1)
> 1: 0 4096 246504 7e4a4f00(0, 94208, 1)
> 2: 0 4096 246505 7e4a4f00(0, 94208, 1)
> 3: 0 4096 246506 7e4a4f00(0, 94208, 1)
The above 4 io vectors belong to one same segment since
they are contineous physically from the 3rd column of PFN,
but the vector 4(the below one) isn't contineous with above 3
vectors, so the following one starts the 2nd segment.
> 4: 0 4096 246538 7e4a4f00(0, 94208, 2)
> 5: 0 4096 246539 7e4a4f00(0, 94208, 2)
> 6: 0 4096 246540 7e4a4f00(0, 94208, 2)
> 7: 0 4096 246541 7e4a4f00(0, 94208, 2)
> 8: 0 4096 246542 7e4a4f00(0, 94208, 2)
> 9: 0 4096 246543 7e4a4f00(0, 94208, 2)
>10: 0 4096 246544 7e4a4f00(0, 94208, 2)
>11: 0 4096 246545 7e4a4f00(0, 94208, 2)
>12: 0 4096 246546 7e4a4f00(0, 94208, 2)
>13: 0 4096 246547 7e4a4f00(0, 94208, 2)
>14: 0 4096 246548 7e4a4f00(0, 94208, 2)
>15: 0 4096 246549 7e4a4f00(0, 94208, 2)
>16: 0 4096 246550 7e4a4f00(0, 94208, 2)
>17: 0 4096 246551 7e4a4f00(0, 94208, 2)
>18: 0 4096 246552 7e4a4f00(0, 94208, 2)
>19: 0 4096 246553 7e4a4f00(0, 94208, 2)
The above 16 vectors are contineous physically, but the segment
size becomes 64K now, so blk_bio_segment_split() should have
seen that and started to split the bio.
>20: 0 4096 246554 7e4a4f00(0, 94208, 2)
>21: 0 4096 246555 7e4a4f00(0, 94208, 2)
>22: 0 4096 246556 7e4a4f00(0, 94208, 2)
Unfortunately the bio isn't splitted at all, that means the following
code is run incorrectly:
if (seg_size + bv.bv_len > queue_max_segment_size(q))
goto new_segment;
seg_size should be 64K, and bv.bv_len should be 4K, so
the sum between the two should be bigger than
64K(queue_max_segment_size(q)). Unfortunately, the
code is optimized as run incorrectly.
> Kernel panic - not syncing: bad block merge
>
> It seems segment 1 is too small and segment 2 too big?
segment 1 is correct, and segment 2 becomes too big, but
__blk_segment_map_sg() runs correctly and figured out
this bio has 3 segments, so causes the oops.
>
> The general plan is to disable inlining (maybe move blk_bio_segment_split()
> to a separate
> function) to try to figure out what is miscompiled.
>
> As you say, this is probably a GCC bug. However, it's likely a middle-end
> or optimization
> bug in the common GCC code.
>
> Dave
>
>
> --
> John David Anglin dave.ang...@bell.net
>
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sg: fix dxferp in from_to case
On Thu, 2016-03-03 at 00:31 -0500, Douglas Gilbert wrote: > This patch is in response to this report: > http://www.spinics.net/lists/linux-scsi/msg93456.html > > One of the strange things that the original sg driver did was let > the user provide both a data-out buffer (it followed the > sg_header+cdb) _and_ specify a reply length greater than zero. What > happened was that the user data-out buffer was copied into some > kernel buffers and then the mid level was told a read type operation > would take place with the data from the device overwriting the same > kernel buffers. The user would then read those kernel buffers back > into the user space. > > From what I can tell, the above action was broken by a change in > 2008 and syzkaller found that out recently. > > ChangeLog: >make sure that a user space pointer is passed through >when data follows the sg_header structure and command. >Fix the abnormal case when a non-zero reply_len is also >given. > > Signed-off-by: Douglas Gilbert This looks correct to me. hp->dxferp used to be set unconditionally, but commit fad7f01e changed it to only be set in the SG_DXFER_TO_DEV case. Reviewed-by: Ewan D. Milne -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] scsi: ufs: add ioctl interface for query request
On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote: > This patch exposes the ioctl interface for UFS driver via SCSI device > ioctl interface. As of now UFS driver would provide the ioctl for query > interface to connected UFS device. > > Reviewed-by: Subhash Jadavani > Signed-off-by: Dolev Raviv > Signed-off-by: Gilad Broner > Signed-off-by: Yaniv Gardi What tool is going to use this ioctl? Why does userspcae want to do something "special" with UFS devices? Shouldn't they just be treated like any other normal block device? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
"scsi: use host wide tags by default" causes uas regression
Hi Christoph,
I've received a bug report that uas is causing a 2-disk enclosure to
lookup with 4.4 and later:
https://bugzilla.redhat.com/show_bug.cgi?id=1315013
Looking at the dmesg this stands out:
Mar 09 01:55:21 larry kernel: sd 2:0:0:1: [sdc] tag#31 uas_eh_abort_handler 0
uas-tag 32 inflight: CMD OUT
Mar 09 01:55:21 larry kernel: sd 2:0:0:1: [sdc] tag#31 CDB: Write(10) 2a 00 e8
10 f4 00 00 04 00 00
Specifically the uas-tag 32, technically that is correct, but we have been
avoiding
actually submitting commands with that tag because bulk-streams are numbered 1-x
and I was afraid that actually using the full-range would trigger off-by-one
errors in devices and it looks I was right.
Before your patch uas was doing:
@@ -929,10 +928,6 @@ static int uas_probe(struct usb_interface *intf, const
struct usb_device_id *id)
if (result)
goto set_alt0;
- result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 2);
- if (result)
- goto free_streams;
-
usb_set_intfdata(intf, shost);
result = scsi_add_host(shost, &intf->dev);
if (result)
With devinfo->qdepth being 32 in this case, so the end-result would be passing
30 to scsi_init_shared_tag_map, reserving one tag for untagged commands and one
tag for the off-by-one thingie.
We also have:
static int uas_slave_configure(struct scsi_device *sdev)
{
...
scsi_change_queue_depth(sdev, devinfo->qdepth - 2);
...
}
But that only helps with single lun enclosures, so I guess that once we fix the
issue that
line can actually go.
So my question is how do I tell the scsi layer to not submit more then X
commands with
scsi_init_shared_tag_map() gone ?
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "scsi: use host wide tags by default" causes uas regression
On Wed, Mar 09, 2016 at 06:58:48PM +0100, Hans de Goede wrote: > So my question is how do I tell the scsi layer to not submit more then X > commands with > scsi_init_shared_tag_map() gone ? By setting ->can_queue in the Scsi_Host structure to the maximum number of command you want outstanding for this host. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "scsi: use host wide tags by default" causes uas regression
Hi, On 09-03-16 19:26, Christoph Hellwig wrote: On Wed, Mar 09, 2016 at 06:58:48PM +0100, Hans de Goede wrote: So my question is how do I tell the scsi layer to not submit more then X commands with scsi_init_shared_tag_map() gone ? By setting ->can_queue in the Scsi_Host structure to the maximum number of command you want outstanding for this host. Can I lower this after calling scsi_host_alloc(), or do I need to make my template "dynamic" ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "scsi: use host wide tags by default" causes uas regression
On Wed, Mar 09, 2016 at 07:28:06PM +0100, Hans de Goede wrote: > >By setting ->can_queue in the Scsi_Host structure to the maximum number > >of command you want outstanding for this host. > > Can I lower this after calling scsi_host_alloc(), or do I need to make > my template "dynamic" ? You don't need to set in the host template, you can set it directly in the Scsi_Host structure. It needs to be set before scsi_add_host, though. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] scsi: ufs: add ioctl interface for query request
> On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote: >> This patch exposes the ioctl interface for UFS driver via SCSI device >> ioctl interface. As of now UFS driver would provide the ioctl for query >> interface to connected UFS device. >> >> Reviewed-by: Subhash Jadavani >> Signed-off-by: Dolev Raviv >> Signed-off-by: Gilad Broner >> Signed-off-by: Yaniv Gardi > > What tool is going to use this ioctl? Why does userspcae want to do > something "special" with UFS devices? Shouldn't they just be treated > like any other normal block device? > Any userspace application can be a tool. We already implemented and used a user space application, that sent queries to the UFS devices in order to get information and descriptors. Not only ioctl interface is a useful way to interact with the device, we used it, and found it very helpful in varies cases. hence, this patch. This patch has been already addressed all comments of Arnd Bergman from 5 months ago, and now, re-uploaded again. thanks, Yaniv > thanks, > > greg k-h > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] scsi: ufs: add ioctl interface for query request
> On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote: >> This patch exposes the ioctl interface for UFS driver via SCSI device >> ioctl interface. As of now UFS driver would provide the ioctl for query >> interface to connected UFS device. >> >> Reviewed-by: Subhash Jadavani >> Signed-off-by: Dolev Raviv >> Signed-off-by: Gilad Broner >> Signed-off-by: Yaniv Gardi > > What tool is going to use this ioctl? Why does userspcae want to do > something "special" with UFS devices? Shouldn't they just be treated > like any other normal block device? > Any userspace application can be a tool. We already implemented and used a user space application, that sent queries to the UFS devices in order to get information and descriptors. Not only ioctl interface is a useful way to interact with the device, we used it, and found it very helpful in varies cases. hence, this patch. This patch has been already addressed all comments of Arnd Bergman from 5 months ago, and now, re-uploaded again. thanks, Yaniv > thanks, > > greg k-h > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] scsi: ufs: add ioctl interface for query request
On Wed, Mar 09, 2016 at 07:09:49PM -, yga...@codeaurora.org wrote: > > On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote: > >> This patch exposes the ioctl interface for UFS driver via SCSI device > >> ioctl interface. As of now UFS driver would provide the ioctl for query > >> interface to connected UFS device. > >> > >> Reviewed-by: Subhash Jadavani > >> Signed-off-by: Dolev Raviv > >> Signed-off-by: Gilad Broner > >> Signed-off-by: Yaniv Gardi > > > > What tool is going to use this ioctl? Why does userspcae want to do > > something "special" with UFS devices? Shouldn't they just be treated > > like any other normal block device? > > > > Any userspace application can be a tool. > We already implemented and used a user space application, that sent > queries to the UFS devices in order to get information and descriptors. But do you want to do with that information? Why does userspace care? > Not only ioctl interface is a useful way to interact with the device, > we used it, and found it very helpful in varies cases. In what case was it helpful? Why does userspace care about ufs specifics, it should just treat it like any other block device and not care at all. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mvsas:Fix possible NULL pointer deference in mvs_dev_found_notify
2016-03-09 15:58 GMT+02:00 Nicholas Krause :
> This adds properly checking after the call to mvs_find_dev_mvi
> due to this function being able to return a NULL pointer and if
> this does arise we will deference it in mvs_alloc_dev due to
> this function never checking if a NULL pointer is given as
> it's input argument.
>
> Signed-off-by: Nicholas Krause
> ---
> drivers/scsi/mvsas/mv_sas.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
> index 83cd3ea..7afb248 100644
> --- a/drivers/scsi/mvsas/mv_sas.c
> +++ b/drivers/scsi/mvsas/mv_sas.c
> @@ -1191,6 +1191,10 @@ int mvs_dev_found_notify(struct domain_device *dev,
> int lock)
> struct mvs_device *mvi_device;
>
> mvi = mvs_find_dev_mvi(dev);
> + if (!mvi) {
> + res = -1;
> + goto found_out;
> + }
>
> if (lock)
> spin_lock_irqsave(&mvi->lock, flags);
> --
> 2.5.0
>
It doesn't look right,
if mvi will be NULL and lock will be set then at
found_out:
if (lock)
spin_unlock_irqrestore(&mvi->lock, flags);
there will be mvi dereference, besides spin_lock_irqsave wasn't even called.
And without this patch dereference would happen on mvi->lock which is
before use in mvs_alloc_dev
About whether mvs_find_dev_mvi can return NULL it looks like it's possible,
but I'm not sure if it practically happens. I guess it did hence patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] scsi: ufs: add ioctl interface for query request
> On Wed, Mar 09, 2016 at 07:09:49PM -, yga...@codeaurora.org wrote: >> > On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote: >> >> This patch exposes the ioctl interface for UFS driver via SCSI device >> >> ioctl interface. As of now UFS driver would provide the ioctl for >> query >> >> interface to connected UFS device. >> >> >> >> Reviewed-by: Subhash Jadavani >> >> Signed-off-by: Dolev Raviv >> >> Signed-off-by: Gilad Broner >> >> Signed-off-by: Yaniv Gardi >> > >> > What tool is going to use this ioctl? Why does userspcae want to do >> > something "special" with UFS devices? Shouldn't they just be treated >> > like any other normal block device? >> > >> >> Any userspace application can be a tool. >> We already implemented and used a user space application, that sent >> queries to the UFS devices in order to get information and descriptors. > > But do you want to do with that information? Why does userspace care? > i don't really understand the subtext of your question - as ANY ioctl cb, we decided to implement the ioctl callback of this scsi device in order to get information like UNIT DESC, DEVICE DESC, FLAGs, ATTRIBUTES. When dealing with UFS devices, one should be able to read the characteristics of the device. why ? well, why not ? during development of this driver, it was useful in many cases to be able to communicate with the device, by simple IOCTL command, rather than implementing ad-hock. regards, Yaniv >> Not only ioctl interface is a useful way to interact with the device, >> we used it, and found it very helpful in varies cases. > > In what case was it helpful? Why does userspace care about ufs > specifics, it should just treat it like any other block device and not > care at all. > > thanks, > > greg k-h > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] "block: make generic_make_request handle arbitrarily sized bios" breaks boot on parisc-linux
On 09.03.2016 16:15, John David Anglin wrote:
> On 2016-03-09 9:43 AM, Ming Lei wrote:
>>> We've provided all the information you asked for, what's the next step
>>> >on this, or do we have to unwind the bio splitting code with reverts
>>> >until it starts working?
>> John, Helge, and I did discuss the problem for a while privately, and looks
>> it is related with compiler. Last time, I sent one patch which can make the
>> issue disappear, but the main change is just invovled with the below:
>>
>> struct bio_vec {
>> struct page*bv_page;
>> -unsigned intbv_len;
>> +unsigned intbv_seg:8;
>> +unsigned intbv_len:24;
>> unsigned intbv_offset;
>> };
>>
>> Maybe John and Helge have some update recently?
>>
>> The logic in blk_bio_segment_split() is correct, and it does respect the max
>> segment size limit.
> Helge has found that tagging blk_bio_segment_split() with "__attribute__
> ((optimize("O0")))"
> makes the issue disappear. The bug remains if one just adds bv_len to the
> struct without the
> bit fields. Maybe problem is evident from following output which I sent to
> Ming and Helge
> last weekend?
>
> blk_rq_map_sg: merge bug: 3 2, extra_len 0, dma_drain 0
> check_bvec: dump bvec for 7e4efdc0(f:2449, t:1)
> 0: 0 4096 246503 7e4a4f00(0, 94208, 1)
> 1: 0 4096 246504 7e4a4f00(0, 94208, 1)
> 2: 0 4096 246505 7e4a4f00(0, 94208, 1)
> 3: 0 4096 246506 7e4a4f00(0, 94208, 1)
> 4: 0 4096 246538 7e4a4f00(0, 94208, 2)
> 5: 0 4096 246539 7e4a4f00(0, 94208, 2)
> 6: 0 4096 246540 7e4a4f00(0, 94208, 2)
> 7: 0 4096 246541 7e4a4f00(0, 94208, 2)
> 8: 0 4096 246542 7e4a4f00(0, 94208, 2)
> 9: 0 4096 246543 7e4a4f00(0, 94208, 2)
>10: 0 4096 246544 7e4a4f00(0, 94208, 2)
>11: 0 4096 246545 7e4a4f00(0, 94208, 2)
>12: 0 4096 246546 7e4a4f00(0, 94208, 2)
>13: 0 4096 246547 7e4a4f00(0, 94208, 2)
>14: 0 4096 246548 7e4a4f00(0, 94208, 2)
>15: 0 4096 246549 7e4a4f00(0, 94208, 2)
>16: 0 4096 246550 7e4a4f00(0, 94208, 2)
>17: 0 4096 246551 7e4a4f00(0, 94208, 2)
>18: 0 4096 246552 7e4a4f00(0, 94208, 2)
>19: 0 4096 246553 7e4a4f00(0, 94208, 2)
>20: 0 4096 246554 7e4a4f00(0, 94208, 2)
>21: 0 4096 246555 7e4a4f00(0, 94208, 2)
>22: 0 4096 246556 7e4a4f00(0, 94208, 2)
> Kernel panic - not syncing: bad block merge
>
> It seems segment 1 is too small and segment 2 too big?
>
> The general plan is to disable inlining (maybe move blk_bio_segment_split()
> to a separate
> function) to try to figure out what is miscompiled.
Right.
I just succeeded in reproducing the bug with moving blk_bio_segment_split()
into an own file
(and with "extern" instead of "static" in blk-merge.c). When compiled with -O2
it still crashes.
So, next step is to analyze what gcc does wrong when compiling this function.
It should get easier now to find the reason, since we have a smaller reproducer
now.
Helge
> As you say, this is probably a GCC bug. However, it's likely a middle-end or
> optimization
> bug in the common GCC code.
>
> Dave
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] scsi: ufs: add ioctl interface for query request
On Wed, Mar 09, 2016 at 08:52:59PM -, yga...@codeaurora.org wrote: > > On Wed, Mar 09, 2016 at 07:09:49PM -, yga...@codeaurora.org wrote: > >> > On Wed, Mar 09, 2016 at 04:11:33PM +0200, Yaniv Gardi wrote: > >> >> This patch exposes the ioctl interface for UFS driver via SCSI device > >> >> ioctl interface. As of now UFS driver would provide the ioctl for > >> query > >> >> interface to connected UFS device. > >> >> > >> >> Reviewed-by: Subhash Jadavani > >> >> Signed-off-by: Dolev Raviv > >> >> Signed-off-by: Gilad Broner > >> >> Signed-off-by: Yaniv Gardi > >> > > >> > What tool is going to use this ioctl? Why does userspcae want to do > >> > something "special" with UFS devices? Shouldn't they just be treated > >> > like any other normal block device? > >> > > >> > >> Any userspace application can be a tool. > >> We already implemented and used a user space application, that sent > >> queries to the UFS devices in order to get information and descriptors. > > > > But do you want to do with that information? Why does userspace care? > > > > i don't really understand the subtext of your question - > as ANY ioctl cb, we decided to implement the ioctl callback of this scsi > device in order to get information like UNIT DESC, DEVICE DESC, FLAGs, > ATTRIBUTES. > When dealing with UFS devices, one should be able to read the > characteristics of the device. why ? well, why not ? Why aren't those characteristics just exported as sysfs attributes under control by the UFS controller driver? Why do you need/want an ioctl for this? > during development of this driver, it was useful in many cases to be able > to communicate with the device, by simple IOCTL command, rather than > implementing ad-hock. Do other storage busses have these types of "custom" ioctls for their bus-type alone? For simple attributes like this, shouldn't you be using sysfs instead so that it is much easier for userspace tools to get access to them? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] "block: make generic_make_request handle arbitrarily sized bios" breaks boot on parisc-linux
On Wed, 2016-03-09 at 22:20 +0100, Helge Deller wrote:
> On 09.03.2016 16:15, John David Anglin wrote:
> > On 2016-03-09 9:43 AM, Ming Lei wrote:
> > > > We've provided all the information you asked for, what's the
> > > > next step
> > > > > on this, or do we have to unwind the bio splitting code with
> > > > > reverts
> > > > > until it starts working?
> > > John, Helge, and I did discuss the problem for a while privately,
> > > and looks
> > > it is related with compiler. Last time, I sent one patch which
> > > can make the
> > > issue disappear, but the main change is just invovled with the
> > > below:
> > >
> > > struct bio_vec {
> > > struct page*bv_page;
> > > -unsigned intbv_len;
> > > +unsigned intbv_seg:8;
> > > +unsigned intbv_len:24;
> > > unsigned intbv_offset;
> > > };
> > >
> > > Maybe John and Helge have some update recently?
> > >
> > > The logic in blk_bio_segment_split() is correct, and it does
> > > respect the max
> > > segment size limit.
> > Helge has found that tagging blk_bio_segment_split() with
> > "__attribute__ ((optimize("O0")))"
> > makes the issue disappear. The bug remains if one just adds bv_len
> > to the struct without the
> > bit fields. Maybe problem is evident from following output which I
> > sent to Ming and Helge
> > last weekend?
> >
> > blk_rq_map_sg: merge bug: 3 2, extra_len 0, dma_drain 0
> > check_bvec: dump bvec for 7e4efdc0(f:2449, t:1)
> > 0: 0 4096 246503 7e4a4f00(0, 94208, 1)
> > 1: 0 4096 246504 7e4a4f00(0, 94208, 1)
> > 2: 0 4096 246505 7e4a4f00(0, 94208, 1)
> > 3: 0 4096 246506 7e4a4f00(0, 94208, 1)
> > 4: 0 4096 246538 7e4a4f00(0, 94208, 2)
> > 5: 0 4096 246539 7e4a4f00(0, 94208, 2)
> > 6: 0 4096 246540 7e4a4f00(0, 94208, 2)
> > 7: 0 4096 246541 7e4a4f00(0, 94208, 2)
> > 8: 0 4096 246542 7e4a4f00(0, 94208, 2)
> > 9: 0 4096 246543 7e4a4f00(0, 94208, 2)
> >10: 0 4096 246544 7e4a4f00(0, 94208, 2)
> >11: 0 4096 246545 7e4a4f00(0, 94208, 2)
> >12: 0 4096 246546 7e4a4f00(0, 94208, 2)
> >13: 0 4096 246547 7e4a4f00(0, 94208, 2)
> >14: 0 4096 246548 7e4a4f00(0, 94208, 2)
> >15: 0 4096 246549 7e4a4f00(0, 94208, 2)
> >16: 0 4096 246550 7e4a4f00(0, 94208, 2)
> >17: 0 4096 246551 7e4a4f00(0, 94208, 2)
> >18: 0 4096 246552 7e4a4f00(0, 94208, 2)
> >19: 0 4096 246553 7e4a4f00(0, 94208, 2)
> >20: 0 4096 246554 7e4a4f00(0, 94208, 2)
> >21: 0 4096 246555 7e4a4f00(0, 94208, 2)
> >22: 0 4096 246556 7e4a4f00(0, 94208, 2)
> > Kernel panic - not syncing: bad block merge
> >
> > It seems segment 1 is too small and segment 2 too big?
> >
> > The general plan is to disable inlining (maybe move
> > blk_bio_segment_split() to a separate
> > function) to try to figure out what is miscompiled.
>
> Right.
> I just succeeded in reproducing the bug with moving
> blk_bio_segment_split() into an own file
> (and with "extern" instead of "static" in blk-merge.c). When compiled
> with -O2 it still crashes.
> So, next step is to analyze what gcc does wrong when compiling this
> function.
> It should get easier now to find the reason, since we have a smaller
> reproducer now.
OK, that would explain why I don't see the problem, since I'm using an
older compiler. So it's our issue and basically no action for block.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mvsas:Fix possible NULL pointer deference in mvs_dev_found_notify
On Wed, 09 Mar 2016 22:33:47 +0200, DÄvis MosÄns said: > About whether mvs_find_dev_mvi can return NULL it looks like it's possible, > but I'm not sure if it practically happens. I guess it did hence patch. Or the "bug" was found by incorrect code inspection. Nick has a history of submitting patches that are for either non-existent problems or don't deal with with the issue correctly - bad enough that he's not allowed to post to the linux-kernel list. pgpvvUrIKYeLq.pgp Description: PGP signature
Re: [PATCH] sg: fix dxferp in from_to case
> "Doug" == Douglas Gilbert writes: Doug> Make sure that a user space pointer is passed through when data Doug> follows the sg_header structure and command. Fix the abnormal Doug> case when a non-zero reply_len is also given. Applied to 4.5/scsi-fixes. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH resend 2/2] mpt3sas: Remove unnecessary synchronize_irq() before free_irq()
> "Lars-Peter" == Lars-Peter Clausen writes: Lars-Peter> Calling synchronize_irq() right before free_irq() is quite Lars-Peter> useless. On one hand the IRQ can easily fire again before Lars-Peter> free_irq() is entered, on the other hand free_irq() itself Lars-Peter> calls synchronize_irq() internally (in a race condition free Lars-Peter> way), before any state associated with the IRQ is freed. Applied to 4.6/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 06/17] scsi: ufs: separate device and host quirks
> "Yaniv" == Yaniv Gardi writes: Yaniv> Currently we use the host quirks mechanism in order to handle Yaniv> both device and host controller quirks. In order to support Yaniv> various of UFS devices we should separate handling the device Yaniv> quirks from the host controller's. This patch causes a circular dependency: depmod: ERROR: Cycle detected: ufshcd -> ufs_quirks -> ufshcd Please fix! -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] sd: do not let LBPME bit stop the VPDs speak
> "Tom" == tom ty89 writes: Tom, Tom> Some devices have details of their support on unmapping on the Tom> Block Limits and/or Logical Block Provisioning VPDs while they do Tom> not set the LBPME bit to 1. Though this is required by the SCSI Tom> standards, the VPDs are giving even more concrete details about the Tom> support, so they should be used even when the bit is set to 0. I am not going to enable an already brittle feature if a device can't report the right thing. Does the bridge report LBPME=1 if you plug in an SSD? -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] sd: do not let LBPME bit stop the VPDs speak
> "Tom" == Tom Yan writes: Tom> Btw, why SD_MAX_WS16_BLOCKS (which is used for both SD_LBP_WS16 and Tom> SD_LBP_UNMAP) is set to 0x7f (23-bit) instead of 0x Tom> (32-bit, 4-byte)? This limit represents the largest block range we can describe using a single bio. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] "block: make generic_make_request handle arbitrarily sized bios" breaks boot on parisc-linux
> Right. > I just succeeded in reproducing the bug with moving blk_bio_segment_split() > into an own file (and with "extern" instead of "static" in blk-merge.c). > When compiled with -O2 it still crashes. So, next step is to analyze what > gcc does wrong when compiling this function. It should get easier now to > find the reason, since we have a smaller reproducer now. I have a ton of compilers here on my C8000 and a few even on my C3600 which drive nightly CMake dashboards. Could you send the testcase so I can pass it through the list and see what breaks? Greetings, Eike signature.asc Description: This is a digitally signed message part.
