Re: [PATCH] sd: Optimal I/O size is in bytes, not sectors
On Wed, Jan 20, 2016 at 08:47:29AM -0800, James Bottomley wrote: > On Wed, 2016-01-20 at 11:40 -0500, Martin K. Petersen wrote: > > > > > > > "James" == James Bottomley < > > > > > > > james.bottom...@hansenpartnership.com> writes: > > > > James> We should mark the commit causing the problems, which went > > into > > James> 4.4 if I remember correctly: > > > > James> Fixes: ca369d51b3e1649be4a72addd6d6a168cfb3f537 Cc: > > James> sta...@vger.kernel.org # 4.4+ Reviewed-by: James E.J. > > Bottomley > > James> > > > > I'll add the tags. The reason I didn't explicitly put 4.4+ is that > > the original commit has made its way quite far in various stable > > trees by now. > > It has? It wasn't tagged for stable. However, if it got applied to > stable trees, then we should certainly backport further. I sort of > hope the stable process uses the Fixes: tag to decide when to backport > anyway, since the stable commit contains the original upstream sha256, > they can certainly identify it. > > Greg, are we OK not to bother with qualifying the cc: stable tag if we > have a Fixes tag, or do you still want to see both? Perhaps an > addition to stable_kernel_rules.txt mentioning Fixes: might be useful > as well. I want to see a stable tag if you know it is relevant. I do, when I have the time, dig through the fixes: tags to see if they should also be applied, but I don't always guarantee that I will get to them at all, so don't count on that as a way to get a patch into a stable release. 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: st driver doesn't seem to grok LTO partitioning
Laurence, Shane Seymour from HP provided this useful link: https://docs.oracle.com/cd/E21419_04/en/LTO5_Vol3_E5b/LTO5_Vol3_E5b.pdf That documents the WRITE BUFFER command on pages 228 and 229 used by HP LTO-5 drives. For sending new firmware it looks like you need to use MODE 4 or 5. [That description does not look like the work of an engineer who understands the subject matter :-)]. And it seems like HP implement the REPORT SUPPORTED OPCODES command which means you can use the sg_opcodes utility. For the WRITE BUFFER command, that should break out the MODE numbers that the drive actually supports. For example, this from a SSD: ... 3b 0 10 Write buffer, combined header and data [or multiple modes] 3b 2 10 Write buffer, data 3b 4 10 Write buffer, download microcode and activate 3b 5 10 Write buffer, download microcode, save, and activate 3b 6 10 Write buffer, download microcode with offsets and activate 3b 7 10 Write buffer, download microcode with offsets, save, and activate 3b a 10 Write buffer, write data to echo buffer ... Looking back in this thread I see you mention a Quantum Ultrium 5 tape drive; so this HP information may not apply. Doug Gilbert On 16-01-14 09:12 PM, Laurence Oberman wrote: All attempts to get my drive and changer firmware updated have failed. So I wont be able to add another "tested by" to this thread unless I can find another drive. Even using Doug's method fails to actually update. Seems to suck up the image and then do nothing. Lands up in getting hung and needs a full power reset. The image I have is the correct image. I dont want to try to many convoluted methods because I dont want to brick the changer. Its the only one I have for doing all the st driver testing here at Red Hat in the GSS team. Thanks Laurence Oberman Principal Software Maintenance Engineer Red Hat Global Support Services - Original Message - From: "Emmanuel Florac" To: "Laurence Oberman" Cc: "Laurence Oberman" , "Kai Makisara" , linux-scsi@vger.kernel.org Sent: Wednesday, January 6, 2016 11:07:20 AM Subject: Re: st driver doesn't seem to grok LTO partitioning Le Wed, 6 Jan 2016 10:23:34 -0500 (EST) Laurence Oberman écrivait: MaxPartitions: 0 Drive is working fine, # mt -f /dev/st0 status SCSI 2 tape drive: File number=0, block number=0, partition=0. Tape block size 512 bytes. Density code 0x58 (no translation). Soft error count since last status=0 General status bits on (4101): BOT ONLINE IM_REP_EN This is what I get when I try and partition and I believe this may be a firmware issue for me. Yes probably, it reports "MaxPartitions 0", should be 1 for an LTO-5 drive. Weird. -- 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
spurious use after free bug in dio_bio_complete on scsi disks (with multipath)
When I enable DEBUG_PAGEALLOC (notice: needs also enablement on the command line) I get spurious warnings like the following [ 2664.756567] Unable to handle kernel pointer dereference in virtual kernel address space [ 2664.756575] failing address: 00f9ef84c000 TEID: 00f9ef84c803 [ 2664.756577] Fault in home space mode while using kernel ASCE. [ 2664.756582] AS:00f8b007 R3:00ff62c03007 S:00ff62887000 P:00f9ef84c400 [ 2664.756618] Oops: 0011 ilc:2 [#1] SMP DEBUG_PAGEALLOC [ 2664.756629] Modules linked in: nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc btrfs xor raid6_pq ghash_s390 prng ecb aes_s390 des_s390 des_generic sha512_s390 sha256_s390 sha1_s390 sha_common eadm_sch nfsd auth_rpcgss oid_registry nfs_acl lockd grace vhost_net tun vhost macvtap macvlan sunrpc dm_service_time dm_multipath dm_mod autofs4 [ 2664.756721] CPU: 33 PID: 0 Comm: swapper/33 Not tainted 4.4.0+ #117 [ 2664.756726] task: 00fa6e86e0a0 ti: 00fa6e87 task.ti: 00fa6e87 [ 2664.756731] Krnl PSW : 0704e0018000 0034efba (dio_bio_complete+0xf2/0x100) [ 2664.756743]R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 EA:3 Krnl GPRS: 00fa6e87 0001 [ 2664.756751]0034efba 00fa59ff2bc8 [ 2664.756754]00fa4e6a5f00 00f9ef84ce00 00fa1000 00fa4e6a5f38 [ 2664.756757]1000 00841a88 0034efba 00fa6de6bbe8 [ 2664.756770] Krnl Code: 0034efac: a784ffb6brc 8,34ef18 0034efb0: b9040029lgr %r2,%r9 #0034efb4: c0e5000f064ebrasl %r14,52fc50 >0034efba: 58c09014l %r12,20(%r9) 0034efbe: a7f4ffecbrc 15,34ef96 0034efc2: 0707bcr 0,%r7 0034efc4: 0707bcr 0,%r7 0034efc6: 0707bcr 0,%r7 [ 2664.756826] Call Trace: [ 2664.756829] ([<0034efba>] dio_bio_complete+0xf2/0x100) [ 2664.756833] [<0034f26a>] dio_bio_end_aio+0x42/0x168 [ 2664.756837] [<00538b0a>] blk_update_request+0x102/0x468 [ 2664.756842] [<00607f20>] scsi_end_request+0x48/0x1d0 [ 2664.756845] [<00609b88>] scsi_io_completion+0x110/0x690 [ 2664.756849] [<00542186>] blk_done_softirq+0xb6/0xd0 [ 2664.756854] [<00165e3c>] __do_softirq+0xd4/0x4b0 [ 2664.756856] [<001665f2>] irq_exit+0xe2/0x100 [ 2664.756859] [<0010ce02>] do_IRQ+0x6a/0x88 [ 2664.756863] [<0081d5e6>] io_int_handler+0x11a/0x25c [ 2664.756867] [<00104900>] enabled_wait+0x58/0xe8 [ 2664.756870] ([<001048e8>] enabled_wait+0x40/0xe8) [ 2664.756875] [<00104da2>] arch_cpu_idle+0x32/0x48 [ 2664.756880] [<001b45a6>] default_idle_call+0x3e/0x58 [ 2664.756883] [<001b481c>] cpu_startup_entry+0x25c/0x358 [ 2664.756887] [<001153ca>] smp_start_secondary+0xf2/0x100 [ 2664.756890] [<0081dbb2>] restart_int_handler+0x62/0x78 [ 2664.756893] [<0081d894>] save_fpu_regs+0xc/0x78 [ 2664.756895] INFO: lockdep is turned off. [ 2664.756898] Last Breaking-Event-Address: [ 2664.756903] [<002f0278>] kmem_cache_free+0x1f8/0x3c0 [ 2664.756907] [ 2664.756911] Kernel panic - not syncing: Fatal exception in interrupt I can reproduce on 4.4 and 4.3 and had not yet the chance to go back to older versions. I can create a dump anytime, so I can look into some data structures if that is of any help. Any ideas or ideas how to debug that further? Christian -- 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: What partition should the MTMKPART argument specify? Was: Re: st driver doesn't seem to grok LTO partitioning
Le Fri, 22 Jan 2016 02:10:03 + "Seymour, Shane M" écrivait: > > However, before making the final patch, we should decide which > > partition the specified size should apply to. For the SCSI level > > <=2 it applies to partition 1. For other drives we may have some > > freedom to “tune” the definition. The size should apply to the > > partition the users expect it to apply. > > I'd argue for consistency here in the current interface and that it > should apply in the same way more on that below. I agree, it would be extremely surprising (in a bad way) to see your application behave differently when upgrading your drive, for instance. > > Partitioning with two partitions is used for storing index in a > > small partition and use the rest of the tape for data. In this > > case, it is probably natural to specify the size of the index. The > > LTFS definition supports index in any partition. The open source > > code I have seen seem to default to index in partition 0. > > > > The HP and IBM LTO default partitioning (FDP=1) specifies two wraps > > (minimum) to partition 1 and the rest to 0. LTFS 2.2 specifications doesn't explicitly number partitions, however it makes it clear that the "index" partition (the smaller one) must come first. Furthermore, all LTFS tapes I've had a look at have partition 0 as index. Add to this the fact that current LTO-7 tapes are 6 TB. That comes to pretty big numbers when you want to size a partition in MB. > It may be worth noting (if you're going to update any documentation) > that isn't 100% accurate. You actually get one wrap in partition 1 > and the rest minus one wrap into partition 0. There is one wrap used > as a guard between the two partitions. The size given to a partition > is rounded up to the size of a wrap as well. > > See > https://docs.oracle.com/cd/E21419_04/en/LTO5_Vol3_E5b/LTO5_Vol3_E5b.pdf > > Page 100 where it gives examples of how partition sizes are > calculated on HP LTO5 drives. > Ican confirm this from the tests I've made on LTO-5 and LTO-6. The size of the partition you'll obtain may end quite different from the size you've asked for. This is made particularly difficult when you must convert a few TB to MB, and you don't know exactly where it'll > > > > There seem to be lot of arguments supporting both possible choices. > > Should we use the existing definition (1) or change it for the > > drives supporting SCSI level >= 3 (or supporting FORMAT MEDIUM)? > > The definition can’t be changed later. This is why we should make a > > good decision. > > > > Opinions? > > How about using the fact the size is signed to indicate one slightly > different thing? I'm not sure if you'd call this using or abusing the > fact that it's signed. > > Make the default behavior for a positive size the same as the current > behavior for SCSI-2 (size applies to partition 1). If the size is > negative then the absolute value of the size applies to partition 0. > That provides some flexibility in choosing which partition the size > applies to if it worked that way for all devices. > > With that you also get consistent behavior between tape drives without > having to know if the size will apply to partition 0 or partition 1 > based on the tape technology and you get the benefit of being able to > set an explicit size for partition 0 or partition 1. > > You could overload the value of 0 as well to use FDP to choose the > sizes for the partitions (assuming 0 doesn't already have a side > effect in the code). Then you get whatever the tape drive wants to do. -- Emmanuel Florac | Direction technique | Intellique | | +33 1 78 94 84 02 -- 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: What partition should the MTMKPART argument specify? Was: Re: st driver doesn't seem to grok LTO partitioning
Ooops, fat finger posting before I've finished answering... Le Fri, 22 Jan 2016 02:10:03 + "Seymour, Shane M" écrivait: > > > > There seem to be lot of arguments supporting both possible choices. > > Should we use the existing definition (1) or change it for the > > drives supporting SCSI level >= 3 (or supporting FORMAT MEDIUM)? > > The definition can’t be changed later. This is why we should make a > > good decision. > > > > Opinions? > > How about using the fact the size is signed to indicate one slightly > different thing? I'm not sure if you'd call this using or abusing the > fact that it's signed. > I find the idea of using negative number as "interesting". I'm afraid of what will happen as tape size grows? Don't we risk overflowing at some point, and ending with very unexpected results? Hmm in fact if we keep using MB we'll be stuck when tapes reach ~2 PB which leaves some time to think about it, until LTO-15 circa 2036 :) I have a different proposition: what about adding a new ioctl named MTMKADVPART that takes a struct to define many different partitions at once? So that we'd be able to create 2 partitions with the existing MTMKPART, and also create several partitions for newer drives? Added bonus a corresponding ioctl to read the media configuration as specified in mode sense page 11h (see LTO SCSI reference) and present it back so that we could know how the tape is actually partitioned :) -- Emmanuel Florac | Direction technique | Intellique | | +33 1 78 94 84 02 -- 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 3/3] xen-scsiback: fix license ident used in MODULE_LICENSE
The comment at the beginning of the file is the canonical source of licenses for this module. Currently it contains GPL and MIT license. Fix the code to reflect the reality. Signed-off-by: Wei Liu --- drivers/xen/xen-scsiback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index ad4eb10..0695cf8 100644 --- a/drivers/xen/xen-scsiback.c +++ b/drivers/xen/xen-scsiback.c @@ -1915,6 +1915,6 @@ module_init(scsiback_init); module_exit(scsiback_exit); MODULE_DESCRIPTION("Xen SCSI backend driver"); -MODULE_LICENSE("Dual BSD/GPL"); +MODULE_LICENSE("Dual MIT/GPL"); MODULE_ALIAS("xen-backend:vscsi"); MODULE_AUTHOR("Juergen Gross "); -- 2.1.4 -- 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] megaraid: fix null pointer check in megasas_detach_one().
The pd_seq_sync pointer can't be NULL, we have to check its entries instead. Signed-off-by: Maurizio Lombardi --- drivers/scsi/megaraid/megaraid_sas_base.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 97a1c1c..f083e74 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -5941,11 +5941,11 @@ static void megasas_detach_one(struct pci_dev *pdev) if (fusion->ld_drv_map[i]) free_pages((ulong)fusion->ld_drv_map[i], fusion->drv_map_pages); - if (fusion->pd_seq_sync) - dma_free_coherent(&instance->pdev->dev, - pd_seq_map_sz, - fusion->pd_seq_sync[i], - fusion->pd_seq_phys[i]); + if (fusion->pd_seq_sync[i]) + dma_free_coherent(&instance->pdev->dev, + pd_seq_map_sz, + fusion->pd_seq_sync[i], + fusion->pd_seq_phys[i]); } free_pages((ulong)instance->ctrl_context, instance->ctrl_context_pages); -- Maurizio Lombardi -- 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 3/3] xen-scsiback: fix license ident used in MODULE_LICENSE
On 22/01/16 13:34, Wei Liu wrote: > The comment at the beginning of the file is the canonical source of > licenses for this module. Currently it contains GPL and MIT license. > Fix the code to reflect the reality. > > Signed-off-by: Wei Liu Acked-by: Juergen Gross > --- > drivers/xen/xen-scsiback.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c > index ad4eb10..0695cf8 100644 > --- a/drivers/xen/xen-scsiback.c > +++ b/drivers/xen/xen-scsiback.c > @@ -1915,6 +1915,6 @@ module_init(scsiback_init); > module_exit(scsiback_exit); > > MODULE_DESCRIPTION("Xen SCSI backend driver"); > -MODULE_LICENSE("Dual BSD/GPL"); > +MODULE_LICENSE("Dual MIT/GPL"); > MODULE_ALIAS("xen-backend:vscsi"); > MODULE_AUTHOR("Juergen Gross "); > -- 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 V3 8/9] aacraid: Fix character device re-initialization
On 20.1.2016 21:43, Raghava Aditya Renukunta wrote: > Hello Tomas, > >> -Original Message- >> From: Tomas Henzl [mailto:the...@redhat.com] >> Sent: Wednesday, January 20, 2016 5:41 AM >> To: Raghava Aditya Renukunta; james.bottom...@hansenpartnership.com; >> martin.peter...@oracle.com; linux-scsi@vger.kernel.org >> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc- >> sierra.com; Scott Benesh; jthumsh...@suse.de; shane.seym...@hpe.com; >> zzzDavid Carroll >> Subject: Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization >> >> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote: >>> From: Raghava Aditya Renukunta >>> >>> During EEH PCI hotplug activity kernel unloads and loads the driver, >>> causing character device to be unregistered(aac_remove_one).When the >>> driver is loaded back using aac_probe_one the character device needs >>> to be registered again for the AIF management tools to work. >>> >>> Fixed by adding code to register character device in aac_probe_one if >>> it is unregistered in aac_remove_one. >>> >>> Changes in V2: >>> Added macros to track character device state >>> >>> Changes in V3: >>> None >>> >>> Signed-off-by: Raghava Aditya Renukunta >> >>> Reviewed-by: Shane Seymour >> Hi Raghava, >> when aacraid is loaded (modprobe) without an controller attached to the >> system >> the driver loads and creates the character device. Later when you hotplug a >> device and remove again we see the driver loaded but now without the >> char device. I'd prefer consistency here - either create the char device >> when the first controller is probed (preferred) or do not remove it >> until the driver exits. >> This is not a nack, just a wish that you changed it in next series. >> >> --tm > > Yes I will make the necessary changes so that character device is created > when > The controller is probed, and when the driver is removed > (aac_remove_one),delete > the character device. I will keep the character device during resume and > suspend. > > Do you want to do this in the next version of the patches or the next series > of patches after this one is > Accepted. ? sure, next series is fine, as I wrote already > > Regards, > Raghava Aditya > > >>> --- >>> drivers/scsi/aacraid/aacraid.h | 7 +++ >>> drivers/scsi/aacraid/linit.c | 21 ++--- >>> 2 files changed, 21 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h >>> index 3473668..4b669ef 100644 >>> --- a/drivers/scsi/aacraid/aacraid.h >>> +++ b/drivers/scsi/aacraid/aacraid.h >>> @@ -94,6 +94,13 @@ enum { >>> #define aac_phys_to_logical(x) ((x)+1) >>> #define aac_logical_to_phys(x) ((x)?(x)-1:0) >>> >>> +/* >>> + * These macros are for keeping track of >>> + * character device state. >>> + */ >>> +#define AAC_CHARDEV_UNREGISTERED (-1) >>> +#define AAC_CHARDEV_NEEDS_REINIT (-2) >>> + >>> /* #define AAC_DETAILED_STATUS_INFO */ >>> >>> struct diskparm >>> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c >>> index 27b3fcd..057c07c 100644 >>> --- a/drivers/scsi/aacraid/linit.c >>> +++ b/drivers/scsi/aacraid/linit.c >>> @@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION); >>> >>> static DEFINE_MUTEX(aac_mutex); >>> static LIST_HEAD(aac_devices); >>> -static int aac_cfg_major = -1; >>> +static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED; >>> char aac_driver_version[] = AAC_DRIVER_FULL_VERSION; >>> >>> /* >>> @@ -1125,6 +1125,13 @@ static void __aac_shutdown(struct aac_dev * >> aac) >>> else if (aac->max_msix > 1) >>> pci_disable_msix(aac->pdev); >>> } >>> +static void aac_init_char(void) >>> +{ >>> + aac_cfg_major = register_chrdev(0, "aac", &aac_cfg_fops); >>> + if (aac_cfg_major < 0) { >>> + pr_err("aacraid: unable to register \"aac\" device.\n"); >>> + } >>> +} >>> >>> static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id >> *id) >>> { >>> @@ -1182,6 +1189,9 @@ static int aac_probe_one(struct pci_dev *pdev, >> const struct pci_device_id *id) >>> shost->max_cmd_len = 16; >>> shost->use_cmd_list = 1; >>> >>> + if (aac_cfg_major == AAC_CHARDEV_NEEDS_REINIT) >>> + aac_init_char(); >>> + >>> aac = (struct aac_dev *)shost->hostdata; >>> aac->base_start = pci_resource_start(pdev, 0); >>> aac->scsi_host_ptr = shost; >>> @@ -1519,7 +1529,7 @@ static void aac_remove_one(struct pci_dev >> *pdev) >>> pci_disable_device(pdev); >>> if (list_empty(&aac_devices)) { >>> unregister_chrdev(aac_cfg_major, "aac"); >>> - aac_cfg_major = -1; >>> + aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT; >>> } >>> } >>> >>> @@ -1681,11 +1691,8 @@ static int __init aac_init(void) >>> if (error < 0) >>> return error; >>> >>> - aac_cfg_major = register_chrdev( 0, "aac", &aac_cfg_fops); >>> - if (aac_cfg_major < 0) { >>> - printk(KERN_WARNING >>> -
Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET
On 20.1.2016 21:32, Raghava Aditya Renukunta wrote: > >> -Original Message- >> From: Tomas Henzl [mailto:the...@redhat.com] >> Sent: Tuesday, January 19, 2016 8:14 AM >> To: Raghava Aditya Renukunta; james.bottom...@hansenpartnership.com; >> martin.peter...@oracle.com; linux-scsi@vger.kernel.org >> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc- >> sierra.com; Scott Benesh; jthumsh...@suse.de; shane.seym...@hpe.com; >> David Carroll >> Subject: Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET >> >> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote: >>> From: Raghava Aditya Renukunta >>> >>> while driver removal is in progress or PCI shutdown is invoked, driver >>> kills AIF aacraid thread, but IOCTL requests from the management tools >>> re-start AIF thread leading to IOP_RESET. >>> >>> Fixed by setting adapter_shutdown flag when PCI shutdown is invoked. >>> >>> Changes in V2: >>> Set adapter_shutdown flag before shutdown command is sent to \ >>> controller >>> >>> Changes in V3: >>> Call aac_send_shut_shutdown first thing in __aac_shutdown >>> Convert adapter_shutdown to atomic_t variable to prevent \ >>> SMP coherency issues(race conditions) >> Hi, >> I don't think that an atomic variable can change it, imagine that >> thread just passed your test in aac_cfg_ioctl and another thread >> enter a bit later the adapter_shutdown so both - an I/O and your shutdown >> code >> will run in parallel. >> Also you need to fix your compat_ioctl path too. >> >> --tm > Hello Tomas, > Yes that would not solve this problem. > I can think of 2 solutions > > 1.Place a delay after setting adapter_shutdown and before sending the actual > shutdown command in aac_send_shutdown. This way any impending commands will > be > processed before the adapter actually receives the shutdown command. Since > management > commands are sync only , shutdown command will be sent last. This solution > uses an > estimation of the delay > > 2.Since commands are sync , place a check in aac_fib_send to block > commands once adapter_shutdown is set(only shutdown command will be sent > thru) This option looks better but I guess you still can find a tiny race window. What do you think about a mutual exclusive access using a mutex, do you think this could work? diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c index 54195a117f..b9505c58db 100644 --- a/drivers/scsi/aacraid/commctrl.c +++ b/drivers/scsi/aacraid/commctrl.c @@ -858,6 +858,8 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg) /* * HBA gets first crack */ + if (dev->adapter_shutdown) + return -EACCES; status = aac_dev_ioctl(dev, cmd, arg); if (status != -ENOTTY) diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c index 0e954e37f0..02535c07a4 100644 --- a/drivers/scsi/aacraid/comminit.c +++ b/drivers/scsi/aacraid/comminit.c @@ -212,6 +212,10 @@ int aac_send_shutdown(struct aac_dev * dev) return -ENOMEM; aac_fib_init(fibctx); + mutex_lock(&aac_mutex); + dev->adapter_shutdown = 1; + mutex_unlock(&aac_mutex); + cmd = (struct aac_close *) fib_data(fibctx); cmd->command = cpu_to_le32(VM_CloseAll); @@ -229,7 +233,6 @@ int aac_send_shutdown(struct aac_dev * dev) /* FIB should be freed only after getting the response from the F/W */ if (status != -ERESTARTSYS) aac_fib_free(fibctx); - dev->adapter_shutdown = 1; if ((dev->pdev->device == PMC_DEVICE_S7 || dev->pdev->device == PMC_DEVICE_S8 || dev->pdev->device == PMC_DEVICE_S9) && diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c index 6944560e22..a87880ab32 100644 --- a/drivers/scsi/aacraid/linit.c +++ b/drivers/scsi/aacraid/linit.c @@ -706,8 +706,9 @@ static long aac_cfg_ioctl(struct file *file, int ret; struct aac_dev *aac; aac = (struct aac_dev *)file->private_data; - if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown) + if (!capable(CAP_SYS_RAWIO)) return -EPERM; + mutex_lock(&aac_mutex); ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg); mutex_unlock(&aac_mutex); > > I am more inclined to go with option 2. > > Regards, > Raghava Aditya > >>> Signed-off-by: Raghava Aditya Renukunta >> >>> --- >>> drivers/scsi/aacraid/aacraid.h | 2 +- >>> drivers/scsi/aacraid/comminit.c | 6 +++--- >>> drivers/scsi/aacraid/linit.c| 15 --- >>> 3 files changed, 12 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h >>> index 2916288..3473668 100644 >>> --- a/drivers/scsi/aacraid/aacraid.h >>> +++ b/drivers/scsi/aacraid/aacraid.h >>> @@ -1234,7 +1234,7 @@ struct aac_dev >>> int msi_enabled;/* MSI/MSI-X enabled */ >>> s
[PATCH] scsi: Allow activation of scsi-mq per-driver
Allow the activation of the scsi-mq feature on a per-driver bassis as opposed to the current stack global (de)activation. This allows us to have setups which can combine "slow" rotational media and fast media on two different HBA types. The following is from a host with rotational disks behind a HP SAS Adapter and a fibre channel array behind a Emulex FC Adapter. The hpsa driver does not support scsi-mq yet (and has rotational disks attached to it), but the lpfc does. This patch allows an optimal combination of the scsi-mq enabled lpfc driver and the hpsa driver which still uses a single queue scsi layer and thus can make use of IO schedulers. host:~ # cat /sys/block/sd?/queue/scheduler noop deadline [cfq] none none none none none none none none none none none none none none none none Signed-off-by: Johannes Thumshirn --- drivers/scsi/fnic/fnic_main.c | 10 ++ drivers/scsi/hosts.c | 2 +- drivers/scsi/lpfc/lpfc_init.c | 8 drivers/scsi/virtio_scsi.c| 11 +++ include/scsi/scsi_host.h | 3 --- 5 files changed, 30 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c index 58ce902..dcb06eb 100644 --- a/drivers/scsi/fnic/fnic_main.c +++ b/drivers/scsi/fnic/fnic_main.c @@ -83,6 +83,15 @@ static unsigned int fnic_max_qdepth = FNIC_DFLT_QUEUE_DEPTH; module_param(fnic_max_qdepth, uint, S_IRUGO|S_IWUSR); MODULE_PARM_DESC(fnic_max_qdepth, "Queue depth to report for each LUN"); +#ifdef CONFIG_SCSI_MQ_DEFAULT +static bool fnic_use_blk_mq = true; +#else +static bool fnic_use_blk_mq = false; +#endif + +module_param_named(use_blk_mq, fnic_use_blk_mq, bool, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(use_blk_mq, "Use blk-mq for fnic"); + static struct libfc_function_template fnic_transport_template = { .frame_send = fnic_send, .lport_set_port_id = fnic_set_port_id, @@ -567,6 +576,7 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent) host->host_no); host->transportt = fnic_fc_transport; + host->use_blk_mq = fnic_use_blk_mq; err = fnic_stats_debugfs_init(fnic); if (err) { diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 94025c5..d64288a 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -479,7 +479,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize) else shost->dma_boundary = 0x; - shost->use_blk_mq = scsi_use_blk_mq && !shost->hostt->disable_blk_mq; + shost->use_blk_mq = scsi_use_blk_mq; device_initialize(&shost->shost_gendev); dev_set_name(&shost->shost_gendev, "host%d", shost->host_no); diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c index db9446c..2ea7704 100644 --- a/drivers/scsi/lpfc/lpfc_init.c +++ b/drivers/scsi/lpfc/lpfc_init.c @@ -86,6 +86,13 @@ static struct scsi_transport_template *lpfc_transport_template = NULL; static struct scsi_transport_template *lpfc_vport_transport_template = NULL; static DEFINE_IDR(lpfc_hba_index); +#ifdef CONFIG_SCSI_MQ_DEFAULT +static bool lpfc_use_blk_mq = true; +#else +static bool lpfc_use_blk_mq = false; +#endif +module_param_named(use_blk_mq, lpfc_use_blk_mq, bool, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(use_blk_mq, "Use blk-mq for lpfc driver"); /** * lpfc_config_port_prep - Perform lpfc initialization prior to config port * @phba: pointer to lpfc hba data structure. @@ -3311,6 +3318,7 @@ lpfc_create_port(struct lpfc_hba *phba, int instance, struct device *dev) shost->this_id = -1; shost->max_cmd_len = 16; shost->nr_hw_queues = phba->cfg_fcp_io_channel; + shost->use_blk_mq = lpfc_use_blk_mq; if (phba->sli_rev == LPFC_SLI_REV4) { shost->dma_boundary = phba->sli4_hba.pc_sli4_params.sge_supp_len-1; diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index 7dbbb29..a6e7294 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -34,6 +34,14 @@ #define VIRTIO_SCSI_EVENT_LEN 8 #define VIRTIO_SCSI_VQ_BASE 2 +#ifdef CONFIG_SCSI_MQ_DEFAULT +static bool virtio_use_blk_mq = true; +#else +static bool virtio_use_blk_mq = false; +#endif +module_param_named(use_blk_mq, virtio_use_blk_mq, bool, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(use_blk_mq, "Use blk-mq for virtio_scsi driver"); + /* Command queue element */ struct virtio_scsi_cmd { struct scsi_cmnd *sc; @@ -976,6 +984,9 @@ static int virtscsi_probe(struct virtio_device *vdev) if (!shost) return -ENOMEM; + if (hostt == &virtscsi_host_template_multi) + shost->use_blk_mq = virtio_use_blk_mq; + sg_elems = virtscsi_config_get(vdev, seg_max) ?: 1; shost->sg_tablesize = sg_elems; vscsi = shost_priv(shost); diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index fcfa3d7..3d6a2de 100644 --
Re: [PATCH] scsi: Allow activation of scsi-mq per-driver
On 01/22/2016 02:41 PM, Johannes Thumshirn wrote: > Allow the activation of the scsi-mq feature on a per-driver bassis as opposed > to the current stack global (de)activation. > > This allows us to have setups which can combine "slow" rotational media and > fast media on two different HBA types. > > The following is from a host with rotational disks behind a HP SAS Adapter and > a fibre channel array behind a Emulex FC Adapter. The hpsa driver does not > support scsi-mq yet (and has rotational disks attached to it), but the lpfc > does. This patch allows an optimal combination of the scsi-mq enabled lpfc > driver and the hpsa driver which still uses a single queue scsi layer and thus > can make use of IO schedulers. > > host:~ # cat /sys/block/sd?/queue/scheduler > noop deadline [cfq] > none > none > none > none > none > none > none > none > none > none > none > none > none > none > none > none > > Signed-off-by: Johannes Thumshirn > --- > drivers/scsi/fnic/fnic_main.c | 10 ++ > drivers/scsi/hosts.c | 2 +- > drivers/scsi/lpfc/lpfc_init.c | 8 > drivers/scsi/virtio_scsi.c| 11 +++ > include/scsi/scsi_host.h | 3 --- > 5 files changed, 30 insertions(+), 4 deletions(-) > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- 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 v2 3/3] xen-scsiback: fix license ident used in MODULE_LICENSE
The comment at the beginning of the file is the canonical source of licenses for this module. The license used to distribute outside of Linux kernel is not BSD license but X11 license. Instead of trying to determine whether X11 license can be mapped into Linux's "MIT" license ident, simply make the code to use "GPL" only. Signed-off-by: Wei Liu --- drivers/xen/xen-scsiback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index ad4eb10..250d70f 100644 --- a/drivers/xen/xen-scsiback.c +++ b/drivers/xen/xen-scsiback.c @@ -1915,6 +1915,6 @@ module_init(scsiback_init); module_exit(scsiback_exit); MODULE_DESCRIPTION("Xen SCSI backend driver"); -MODULE_LICENSE("Dual BSD/GPL"); +MODULE_LICENSE("GPL"); MODULE_ALIAS("xen-backend:vscsi"); MODULE_AUTHOR("Juergen Gross "); -- 2.1.4 -- 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, RESEND 2] qla2xxx: Remove use of 'struct timeval'
struct register_host_info stores a 64-bit UTC system time timestamp. This patch removes the use of 'struct timeval' to obtain that timestamp as its tv_sec value will overflow on 32-bit systems in year 2038 beyond. The patch uses ktime_get_real_seconds() which returns a 64-bit seconds value. Signed-off-by: Tina Ruchandani Reviewed-by: Johannes Thumshirn Signed-off-by: Arnd Bergmann diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c index b5029e543b91..15dff7099955 100644 --- a/drivers/scsi/qla2xxx/qla_mr.c +++ b/drivers/scsi/qla2xxx/qla_mr.c @@ -6,6 +6,7 @@ */ #include "qla_def.h" #include +#include #include #include #include @@ -1812,7 +1813,6 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type) struct host_system_info *phost_info; struct register_host_info *preg_hsi; struct new_utsname *p_sysid = NULL; - struct timeval tv; sp = qla2x00_get_sp(vha, fcport, GFP_KERNEL); if (!sp) @@ -1886,8 +1886,7 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type) p_sysid->domainname, DOMNAME_LENGTH); strncpy(phost_info->hostdriver, QLA2XXX_VERSION, VERSION_LENGTH); - do_gettimeofday(&tv); - preg_hsi->utc = (uint64_t)tv.tv_sec; + preg_hsi->utc = (uint64_t)ktime_get_real_seconds(); ql_dbg(ql_dbg_init, vha, 0x0149, "ISP%04X: Host registration with firmware\n", ha->pdev->device); -- 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] scsi_dh_rdac: always retry MODE SELECT on command lock violation
If MODE SELECT returns with sense '05/91/36' (command lock violation) it should always be retried without counting the number of retries. During an HBA upgrade or similar circumstances one might see a flood of MODE SELECT command from various HBAs, which will easily trigger the sense code and exceed the retry count. References: bsc#956949 Signed-off-by: Hannes Reinecke --- drivers/scsi/device_handler/scsi_dh_rdac.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c index 3613581..93880ed 100644 --- a/drivers/scsi/device_handler/scsi_dh_rdac.c +++ b/drivers/scsi/device_handler/scsi_dh_rdac.c @@ -562,7 +562,7 @@ static int mode_select_handle_sense(struct scsi_device *sdev, /* * Command Lock contention */ - err = SCSI_DH_RETRY; + err = SCSI_DH_IMM_RETRY; break; default: break; @@ -612,6 +612,8 @@ retry: err = mode_select_handle_sense(sdev, h->sense); if (err == SCSI_DH_RETRY && retry_cnt--) goto retry; + if (err == SCSI_DH_IMM_RETRY) + goto retry; } if (err == SCSI_DH_OK) { h->state = RDAC_STATE_ACTIVE; -- 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
Re: [PATCH] scsi_dh_rdac: always retry MODE SELECT on command lock violation
On Fri, Jan 22, 2016 at 03:42:41PM +0100, Hannes Reinecke wrote: > If MODE SELECT returns with sense '05/91/36' (command lock violation) > it should always be retried without counting the number of retries. > During an HBA upgrade or similar circumstances one might see a flood > of MODE SELECT command from various HBAs, which will easily trigger > the sense code and exceed the retry count. > > References: bsc#956949 Without the bugzilla refrence Reviewed-by: Johannes Thumshirn > > Signed-off-by: Hannes Reinecke > --- > drivers/scsi/device_handler/scsi_dh_rdac.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c > b/drivers/scsi/device_handler/scsi_dh_rdac.c > index 3613581..93880ed 100644 > --- a/drivers/scsi/device_handler/scsi_dh_rdac.c > +++ b/drivers/scsi/device_handler/scsi_dh_rdac.c > @@ -562,7 +562,7 @@ static int mode_select_handle_sense(struct scsi_device > *sdev, > /* >* Command Lock contention >*/ > - err = SCSI_DH_RETRY; > + err = SCSI_DH_IMM_RETRY; > break; > default: > break; > @@ -612,6 +612,8 @@ retry: > err = mode_select_handle_sense(sdev, h->sense); > if (err == SCSI_DH_RETRY && retry_cnt--) > goto retry; > + if (err == SCSI_DH_IMM_RETRY) > + goto retry; > } > if (err == SCSI_DH_OK) { > h->state = RDAC_STATE_ACTIVE; > -- > 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 -- 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: [Y2038] [PATCH, RESEND 2] qla2xxx: Remove use of 'struct timeval'
On Friday 22 January 2016 15:28:32 Arnd Bergmann wrote: > struct register_host_info stores a 64-bit UTC system time timestamp. > This patch removes the use of 'struct timeval' to obtain that timestamp > as its tv_sec value will overflow on 32-bit systems in year 2038 beyond. > The patch uses ktime_get_real_seconds() which returns a 64-bit seconds value. > > Signed-off-by: Tina Ruchandani > Reviewed-by: Johannes Thumshirn > Signed-off-by: Arnd Bergmann > I was missing From: Tina Ruchandani If anyone is going to pick this up, let me know whether you will fix it up yourself or if I should resend. -- 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] iscsi: fix regression caused by session lock patch
On 11/11/2015 11:05 PM, mchri...@redhat.com wrote: > From: Mike Christie > > This patch fixes this oops report by Guilherme Piccol: > > list_del corruption. prev->next should be c00f3da2b080, but was > c00f3da2c080 Hi Mike! I haven't seen any follow ups on this for a while. What is the plan for this one? Thanks, Brian -- Brian King Power Linux I/O IBM Linux Technology Center -- 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] iscsi: fix regression caused by session lock patch
On 01/22/2016 10:50 AM, Brian King wrote: > On 11/11/2015 11:05 PM, mchri...@redhat.com wrote: >> From: Mike Christie >> >> This patch fixes this oops report by Guilherme Piccol: >> >> list_del corruption. prev->next should be c00f3da2b080, but was >> c00f3da2c080 > > Hi Mike! I haven't seen any follow ups on this for a while. What is the plan > for this one? > Hey, I ended up sending Guilherme a patch for the problem I described here. They were not hitting that problem. Checked if we were hitting a different regression in the abort path. Not hitting that either. Or was still debugging it. -- 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/2] iscsi_ibft: Always display netmask
From: Hannes Reinecke Some older user-space code might rely on the netmask attribute being present, so we should always display it. This fixes a regression introduced by commit 0b2eb3c4060a16f3ec11a4d6d4c934e7e5d5334f. Signed-off-by: Hannes Reinecke Signed-off-by: Lee Duncan --- drivers/firmware/iscsi_ibft.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c index 2dd1fbb8..81037e5fe301 100644 --- a/drivers/firmware/iscsi_ibft.c +++ b/drivers/firmware/iscsi_ibft.c @@ -464,14 +464,8 @@ static umode_t ibft_check_nic_for(void *data, int type) rc = S_IRUGO; break; case ISCSI_BOOT_ETH_PREFIX_LEN: - if (nic->subnet_mask_prefix) - rc = S_IRUGO; - break; case ISCSI_BOOT_ETH_SUBNET_MASK: - if (!memcmp(nic->ip_addr, nulls, 10) && - (nic->ip_addr[10] == 0xff) && - (nic->ip_addr[11] == 0xff) && - nic->subnet_mask_prefix) + if (nic->subnet_mask_prefix) rc = S_IRUGO; break; case ISCSI_BOOT_ETH_ORIGIN: -- 1.8.5.2 -- 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 0/2] RESEND: Enable iBFT IPv6 booting with prefix length
[Resending. Apologies if you get this twice.] It turns out that IPv6 doesn't care about netmask, but instead uses "prefix length" to determine the subnet, but the iBFT subsystem still just supports netmask. These two patches enable IPv6 iBFT (iscsi booting) by adding support for IPv6 prefix and prefix length. Open-iscsi changes to use these new values will be introduced once these changes are present. Hannes Reinecke (2): iscsi_ibft: Add prefix-len attribute iscsi_ibft: Always display netmask drivers/firmware/iscsi_ibft.c| 4 drivers/scsi/iscsi_boot_sysfs.c | 5 + include/linux/iscsi_boot_sysfs.h | 1 + 3 files changed, 10 insertions(+) -- 1.8.5.2 -- 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/2] iscsi_ibft: Add prefix-len attribute
From: Hannes Reinecke The iBFT table only specifies a prefix length, not a netmask. And the netmask is pretty much pointless for IPv6. So introduce a new attribute 'prefix-len' and display the netmask attribute only for IPv4 addresses. Signed-off-by: Hannes Reinecke Signed-off-by: Lee Duncan --- drivers/firmware/iscsi_ibft.c| 12 +++- drivers/scsi/iscsi_boot_sysfs.c | 5 + include/linux/iscsi_boot_sysfs.h | 1 + 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c index 72791232e46b..2dd1fbb8 100644 --- a/drivers/firmware/iscsi_ibft.c +++ b/drivers/firmware/iscsi_ibft.c @@ -319,6 +319,9 @@ static ssize_t ibft_attr_show_nic(void *data, int type, char *buf) val = cpu_to_be32(~((1 << (32-nic->subnet_mask_prefix))-1)); str += sprintf(str, "%pI4", &val); break; + case ISCSI_BOOT_ETH_PREFIX_LEN: + str += sprintf(str, "%d\n", nic->subnet_mask_prefix); + break; case ISCSI_BOOT_ETH_ORIGIN: str += sprintf(str, "%d\n", nic->origin); break; @@ -460,10 +463,17 @@ static umode_t ibft_check_nic_for(void *data, int type) if (address_not_null(nic->ip_addr)) rc = S_IRUGO; break; - case ISCSI_BOOT_ETH_SUBNET_MASK: + case ISCSI_BOOT_ETH_PREFIX_LEN: if (nic->subnet_mask_prefix) rc = S_IRUGO; break; + case ISCSI_BOOT_ETH_SUBNET_MASK: + if (!memcmp(nic->ip_addr, nulls, 10) && + (nic->ip_addr[10] == 0xff) && + (nic->ip_addr[11] == 0xff) && + nic->subnet_mask_prefix) + rc = S_IRUGO; + break; case ISCSI_BOOT_ETH_ORIGIN: rc = S_IRUGO; break; diff --git a/drivers/scsi/iscsi_boot_sysfs.c b/drivers/scsi/iscsi_boot_sysfs.c index 680bf6f0ce76..8f0ea97cf31f 100644 --- a/drivers/scsi/iscsi_boot_sysfs.c +++ b/drivers/scsi/iscsi_boot_sysfs.c @@ -166,6 +166,7 @@ static struct attribute_group iscsi_boot_target_attr_group = { iscsi_boot_rd_attr(eth_index, index, ISCSI_BOOT_ETH_INDEX); iscsi_boot_rd_attr(eth_flags, flags, ISCSI_BOOT_ETH_FLAGS); iscsi_boot_rd_attr(eth_ip, ip-addr, ISCSI_BOOT_ETH_IP_ADDR); +iscsi_boot_rd_attr(eth_prefix, prefix-len, ISCSI_BOOT_ETH_PREFIX_LEN); iscsi_boot_rd_attr(eth_subnet, subnet-mask, ISCSI_BOOT_ETH_SUBNET_MASK); iscsi_boot_rd_attr(eth_origin, origin, ISCSI_BOOT_ETH_ORIGIN); iscsi_boot_rd_attr(eth_gateway, gateway, ISCSI_BOOT_ETH_GATEWAY); @@ -181,6 +182,7 @@ static struct attribute *ethernet_attrs[] = { &iscsi_boot_attr_eth_index.attr, &iscsi_boot_attr_eth_flags.attr, &iscsi_boot_attr_eth_ip.attr, + &iscsi_boot_attr_eth_prefix.attr, &iscsi_boot_attr_eth_subnet.attr, &iscsi_boot_attr_eth_origin.attr, &iscsi_boot_attr_eth_gateway.attr, @@ -208,6 +210,9 @@ static umode_t iscsi_boot_eth_attr_is_visible(struct kobject *kobj, else if (attr == &iscsi_boot_attr_eth_ip.attr) return boot_kobj->is_visible(boot_kobj->data, ISCSI_BOOT_ETH_IP_ADDR); + else if (attr == &iscsi_boot_attr_eth_prefix.attr) + return boot_kobj->is_visible(boot_kobj->data, +ISCSI_BOOT_ETH_PREFIX_LEN); else if (attr == &iscsi_boot_attr_eth_subnet.attr) return boot_kobj->is_visible(boot_kobj->data, ISCSI_BOOT_ETH_SUBNET_MASK); diff --git a/include/linux/iscsi_boot_sysfs.h b/include/linux/iscsi_boot_sysfs.h index 2a8b1659bf35..548d55395488 100644 --- a/include/linux/iscsi_boot_sysfs.h +++ b/include/linux/iscsi_boot_sysfs.h @@ -23,6 +23,7 @@ enum iscsi_boot_eth_properties_enum { ISCSI_BOOT_ETH_INDEX, ISCSI_BOOT_ETH_FLAGS, ISCSI_BOOT_ETH_IP_ADDR, + ISCSI_BOOT_ETH_PREFIX_LEN, ISCSI_BOOT_ETH_SUBNET_MASK, ISCSI_BOOT_ETH_ORIGIN, ISCSI_BOOT_ETH_GATEWAY, -- 1.8.5.2 -- 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
[GIT PULL] final round of SCSI updates for the 4.4+ merge window
This is mostly stuff which missed the first pull request because it needed to incubate longer. It's mainly made up of the ncr 5380 rework but also has a few assorted bug fixes. The patch is available here: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-misc The short changelog is: Dan Carpenter (2): storvsc: Fix typo in MODULE_PARM_DESC cxgbi: Typo in MODULE_PARM_DESC Finn Thain (71): ncr5380: Cleanup whitespace and parentheses atari_NCR5380: Merge changes from NCR5380.c ncr5380: Merge changes from atari_NCR5380.c ncr5380: Fix whitespace in comments using regexp ncr5380: Fix trailing whitespace using regexp ncr5380: Cleanup comments ncr5380: Fix soft lockups atari_scsi, sun3_scsi: Remove global Scsi_Host pointer atari_NCR5380: Eliminate HOSTNO macro atari_NCR5380: Remove HOSTNO macro from printk() and seq_printf() calls ncr5380: Implement new eh_bus_reset_handler ncr5380: Fix EH during arbitration and selection ncr5380: Implement new eh_abort_handler ncr5380: Fix autosense bugs ncr5380: Refactor command completion ncr5380: Use standard list data structure ncr5380: Remove redundant volatile qualifiers ncr5380: Remove LIST and REMOVE macros ncr5380: Use dsprintk() for queue debugging ncr5380: Use shost_priv helper ncr5380: Remove H_NO macro and introduce dsprintk ncr5380: Remove command list debug code ncr5380: Change instance->host_lock to hostdata->lock ncr5380: Remove redundant ICR_ARBITRATION_LOST test and eliminate FLAG_DTC3181E atari_NCR5380: Fix queue_size limit ncr5380: Fix and cleanup scsi_host_template initializers ncr5380: Fix NDEBUG_NO_DATAOUT flag ncr5380: Cleanup #include directives ncr5380: Fix off-by-one bug in extended_msg[] bounds check ncr5380: Standardize reselection handling ncr5380: Replace READ_OVERRUNS macro with FLAG_NO_DMA_FIXUPS ncr5380: Replace redundant flags with FLAG_NO_DMA_FIXUP ncr5380: Introduce NCR5380_poll_politely2 ncr5380: Standardize interrupt handling ncr5380: Remove UNSAFE macro ncr5380: Standardize work queueing algorithm ncr5380: Use work_struct instead of delayed_work ncr5380: Dont wait for BUS FREE after disconnect atari_NCR5380: Use arbitration timeout atari_NCR5380: Set do_abort() timeouts ncr5380: Fix bus phase in do_abort() ncr5380: Fix !REQ timeout in do_abort() ncr5380: Add missing break after case MESSAGE_REJECT ncr5380: Drop DEF_SCSI_QCMD macro ncr5380: Add missing lock in eh_abort_handler ncr5380: Fix NCR5380_transfer_pio() result ncr5380: Rework disconnect versus poll logic ncr5380: Implement NCR5380_dma_xfer_len and remove LIMIT_TRANSFERSIZE macro ncr5380: Always retry arbitration and selection ncr5380: Eliminate selecting state ncr5380: Sleep when polling, if possible ncr5380: Introduce unbound workqueue ncr5380: Cleanup bogus {request,release}_region() calls ncr5380: Eliminate USLEEP_WAITLONG delay ncr5380: Keep BSY asserted when entering SELECTION phase ncr5380: Proceed with next command after NCR5380_select() calls scsi_done ncr5380: Always escalate bad target time-out in NCR5380_select() ncr5380: Use return instead of goto in NCR5380_select() ncr5380: Remove redundant register writes ncr5380: Remove unused hostdata->aborted flag ncr5380: Simplify bus reset handlers atari_NCR5380: Remove RESET_BOOT, CONFIG_ATARI_SCSI_TOSHIBA_DELAY and CONFIG_ATARI_SCSI_RESET_BOOT atari_NCR5380: Reset bus on driver initialization if required ncr5380: Move NCR53C400-specific code ncr5380: Split NCR5380_init() into two functions ncr5380: Remove NCR5380_instance_name macro ncr5380: Remove NCR5380_local_declare and NCR5380_setup macros ncr5380: Remove more pointless macros ncr5380: Eliminate PDEBUG*, TDEBUG* and DTCDEBUG* macros ncr5380: Remove redundant static variable initializers atari_scsi: Fix SCSI host ID setting Hannes Reinecke (1): ncr5380: Remove references to linked commands Insu Yun (1): ipr: Fix out-of-bounds null overwrite John Garry (3): hisi_sas: Use u64 for qw0 in free_device_v1_hw() hisi_sas: Fix typo in setup_itct_v1_hw() hisi_sas: Fix v1 itct masks Mike Christie (1): scsi: add Synology to 1024 sector blacklist Nicholas Krause (1): megaraid: Fix possible NULL pointer deference in mraid_mm_ioctl Ondrej Zary (6): ncr5380: Add support for HP C2502 ncr5380: Fix wait for 53C80 registers registers after PDMA ncr5380: Enable PDMA for DTC chips ncr5380: Enable PDMA for NCR53C400A ncr5380: Use runtime register mapping ncr5380: Fix pseudo DMA transfers on 53C400 Ryan C. Underwood (1): 3w-: Pass through compat mode
Re: [PATCH] scsi: export function scsi_scan.c:sanitize_inquiry_string
On 01/20/2016 01:41 PM, Don Brace wrote: The hpsa driver uses this function to cleanup inquiry data. Our new pqi driver will also use this function. This function was copied into both drivers. This patch exports sanitize_inquiry_string so the hpsa and the pqi drivers can use this function directly. Hannes recommended the change in review: https://www.marc.info/?l=linux-scsi&m=144619249003064&w=2 that using the existing function in scsi_scan would be preferrable. Suggested-by: Hannes Reinecke Reviewed-by: Kevin Barnett Reviewed-by: Justin Lindley Reviewed-by: Scott Teel Reviewed-by: Hannes Reinecke Signed-off-by: Don Brace --- drivers/scsi/scsi_scan.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 6a82066..1f02e84 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -518,7 +518,8 @@ void scsi_target_reap(struct scsi_target *starget) It was pointed out to me by Kevin Barnett that I should also add a prototype in scsi.h. Any thoughts for a V1 with this addition? -- 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] mptlan: add checks for dma mapping errors
mpt_lan_sdu_send() and mpt_lan_post_receive_buckets() do not check if mapping dma memory succeed. The patch adds the checks and failure handling. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov --- drivers/message/fusion/mptlan.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c index cbe96072a6cc..076bf45a95fe 100644 --- a/drivers/message/fusion/mptlan.c +++ b/drivers/message/fusion/mptlan.c @@ -734,6 +734,12 @@ mpt_lan_sdu_send (struct sk_buff *skb, struct net_device *dev) dma = pci_map_single(mpt_dev->pcidev, skb->data, skb->len, PCI_DMA_TODEVICE); + if (dma_mapping_error(mpt_dev->pcidev, dma)) { + netif_stop_queue(dev); + + printk (KERN_ERR "%s: dma mapping failed\n", __func__); + return NETDEV_TX_BUSY; + } priv->SendCtl[ctx].skb = skb; priv->SendCtl[ctx].dma = dma; @@ -1232,6 +1238,14 @@ mpt_lan_post_receive_buckets(struct mpt_lan_priv *priv) dma = pci_map_single(mpt_dev->pcidev, skb->data, len, PCI_DMA_FROMDEVICE); + if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) { + printk (KERN_WARNING + MYNAM "/%s: dma mapping failed\n", + __func__); + priv->mpt_rxfidx[++priv->mpt_rxfidx_tail] = ctx; + spin_unlock_irqrestore(&priv->rxfidx_lock, flags); + break; + } priv->RcvCtl[ctx].skb = skb; priv->RcvCtl[ctx].dma = dma; -- 1.9.1 -- 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] mptlan: add checks for dma mapping errors
Hi Alexey, [auto build test WARNING on v4.4-rc8] [also build test WARNING on next-20160122] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Alexey-Khoroshilov/mptlan-add-checks-for-dma-mapping-errors/20160123-070633 config: xtensa-allyesconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=xtensa All warnings (new ones prefixed by >>): drivers/message/fusion/mptlan.c: In function 'mpt_lan_sdu_send': >> drivers/message/fusion/mptlan.c:737:6: warning: passing argument 1 of >> 'dma_mapping_error' from incompatible pointer type if (dma_mapping_error(mpt_dev->pcidev, dma)) { ^ In file included from arch/xtensa/include/asm/dma-mapping.h:33:0, from include/linux/dma-mapping.h:87, from include/linux/skbuff.h:34, from include/linux/if_ether.h:23, from include/uapi/linux/ethtool.h:17, from include/linux/ethtool.h:16, from include/linux/netdevice.h:42, from drivers/message/fusion/mptlan.h:58, from drivers/message/fusion/mptlan.c:55: include/asm-generic/dma-mapping-common.h:316:19: note: expected 'struct device *' but argument is of type 'struct pci_dev *' static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr) ^ vim +/dma_mapping_error +737 drivers/message/fusion/mptlan.c 721 ctx = priv->mpt_txfidx[priv->mpt_txfidx_tail--]; 722 spin_unlock_irqrestore(&priv->txfidx_lock, flags); 723 724 // dioprintk((KERN_INFO MYNAM ": %s/%s: Creating new msg frame (send).\n", 725 // IOC_AND_NETDEV_NAMES_s_s(dev))); 726 727 pSendReq = (LANSendRequest_t *) mf; 728 729 /* Set the mac.raw pointer, since this apparently isn't getting 730 * done before we get the skb. Pull the data pointer past the mac data. 731 */ 732 skb_reset_mac_header(skb); 733 skb_pull(skb, 12); 734 735 dma = pci_map_single(mpt_dev->pcidev, skb->data, skb->len, 736 PCI_DMA_TODEVICE); > 737 if (dma_mapping_error(mpt_dev->pcidev, dma)) { 738 netif_stop_queue(dev); 739 740 printk (KERN_ERR "%s: dma mapping failed\n", __func__); 741 return NETDEV_TX_BUSY; 742 } 743 744 priv->SendCtl[ctx].skb = skb; 745 priv->SendCtl[ctx].dma = dma; --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
[PATCH v2] mptlan: add checks for dma mapping errors
mpt_lan_sdu_send() and mpt_lan_post_receive_buckets() do not check if mapping dma memory succeed. The patch adds the checks and failure handling. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov --- drivers/message/fusion/mptlan.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c index cbe96072a6cc..3b6c8a755713 100644 --- a/drivers/message/fusion/mptlan.c +++ b/drivers/message/fusion/mptlan.c @@ -734,6 +734,12 @@ mpt_lan_sdu_send (struct sk_buff *skb, struct net_device *dev) dma = pci_map_single(mpt_dev->pcidev, skb->data, skb->len, PCI_DMA_TODEVICE); + if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) { + netif_stop_queue(dev); + + printk (KERN_ERR "%s: dma mapping failed\n", __func__); + return NETDEV_TX_BUSY; + } priv->SendCtl[ctx].skb = skb; priv->SendCtl[ctx].dma = dma; @@ -1232,6 +1238,14 @@ mpt_lan_post_receive_buckets(struct mpt_lan_priv *priv) dma = pci_map_single(mpt_dev->pcidev, skb->data, len, PCI_DMA_FROMDEVICE); + if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) { + printk (KERN_WARNING + MYNAM "/%s: dma mapping failed\n", + __func__); + priv->mpt_rxfidx[++priv->mpt_rxfidx_tail] = ctx; + spin_unlock_irqrestore(&priv->rxfidx_lock, flags); + break; + } priv->RcvCtl[ctx].skb = skb; priv->RcvCtl[ctx].dma = dma; -- 1.9.1 -- 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-v2 0/3] target: Fix LUN_RESET active I/O + TMR handling
From: Nicholas Bellinger Hi folks, Here is -v2 series to address two LUN_RESET active I/O + TMR se_cmd->cmd_kref < 0 bugs as reported recently by Quinn & Co. This bug can occur during active I/O remote port TMR LUN_RESET with multi-port LIO configurations. To address this bug, it adds __target_check_io_state() common handler for ABORT_TASK + LUN_RESET I/O abort cases, and moves remaining se_cmd SGL page + release into target_free_cmd_mem() to now be called directly from the final target_release_cmd_kref() callback. Following __target_check_io_state(), it also obtains kref_get_unless_zero(&se_cmd->cmd_kref) for TMR descs during LUN_RESET in core_tmr_drain_tmr_list(), in order to utilize common transport_wait_for_tasks() code when se_cmd + se_tmr_req descriptors are being aborted or shutdown. At this point the changes are able to survive multi-port active I/O LUN_RESET using iscsi-target ports against Linux + ESX hosts with TAS=1. Note there is still a known issue wrt multi-port active I/O LUN_RESET when se_session shutdown occurs at the same time, that I'm still groking and will need to be addressed for -v3 code. Quinn + Co, please have a look at qla2xxx + TAS with this series when you've got a moment, and let us know how active I/O LUN_RESET works with your test-case. Thank you, --nab v2 changes: - Fix TAS handling for multi-session se_node_acls - Change target_complete_cmd() to check for ABORTED || STOP - Avoid calling target_remove_from_state_list() with se_cmd->t_state_lock held in transport_cmd_check_stop() - Add missing target_free_cmd_mem() in target_put_sess_cmd() special case. - Add a more useful __target_check_io_state() comment - Use assert_spin_locked() + WARN_ON_ONCE() when necessary - Drop unnecessary !list_empty check Nicholas Bellinger (3): target: Fix LUN_RESET active I/O handling for ACK_KREF target: Fix LUN_RESET active TMR descriptor handling target: Fix TAS handling for multi-session se_node_acls drivers/target/target_core_tmr.c | 104 - drivers/target/target_core_transport.c | 73 ++- include/target/target_core_base.h | 1 + 3 files changed, 122 insertions(+), 56 deletions(-) -- 1.9.1 -- 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-v2 1/3] target: Fix LUN_RESET active I/O handling for ACK_KREF
From: Nicholas Bellinger This patch fixes a NULL pointer se_cmd->cmd_kref < 0 refcount bug during TMR LUN_RESET with active se_cmd I/O, that can be triggered during se_cmd descriptor shutdown + release via core_tmr_drain_state_list() code. To address this bug, add common __target_check_io_state() helper for ABORT_TASK + LUN_RESET w/ CMD_T_COMPLETE checking, and set CMD_T_ABORTED + obtain ->cmd_kref for both cases ahead of last target_put_sess_cmd() after TFO->aborted_task() -> transport_cmd_finish_abort() callback has completed. It also introduces SCF_ACK_KREF to determine when transport_cmd_finish_abort() needs to drop the second extra reference, ahead of calling target_put_sess_cmd() for the final kref_put(&se_cmd->cmd_kref). It also updates transport_cmd_check_stop() to avoid holding se_cmd->t_state_lock while dropping se_cmd device state via target_remove_from_state_list(), now that core_tmr_drain_state_list() is holding the se_device lock while checking se_cmd state from within TMR logic. Finally, move transport_put_cmd() release of SGL + TMR + extended CDB memory into target_free_cmd_mem() in order to avoid potential resource leaks in TMR ABORT_TASK + LUN_RESET code-paths. Also update target_release_cmd_kref() accordingly. Cc: Quinn Tran Cc: Himanshu Madhani Cc: Sagi Grimberg Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_tmr.c | 66 -- drivers/target/target_core_transport.c | 56 ++--- include/target/target_core_base.h | 1 + 3 files changed, 75 insertions(+), 48 deletions(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 7137042..5f315b4 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -107,6 +107,34 @@ static int target_check_cdb_and_preempt(struct list_head *list, return 1; } +static bool __target_check_io_state(struct se_cmd *se_cmd) +{ + struct se_session *sess = se_cmd->se_sess; + + assert_spin_locked(&se_session->sess_cmd_lock); + WARN_ON_ONCE(!irqs_disabled()); + /* +* If command already reached CMD_T_COMPLETE state within +* target_complete_cmd(), this se_cmd has been passed to +* fabric driver and will not be aborted. +* +* Otherwise, obtain a local se_cmd->cmd_kref now for TMR +* ABORT_TASK + LUN_RESET for CMD_T_ABORTED processing as +* long as se_cmd->cmd_kref is still active unless zero. +*/ + spin_lock(&se_cmd->t_state_lock); + if (se_cmd->transport_state & CMD_T_COMPLETE) { + pr_debug("Attempted to abort io tag: %llu already complete," + " skipping\n", se_cmd->tag); + spin_unlock(&se_cmd->t_state_lock); + return false; + } + se_cmd->transport_state |= CMD_T_ABORTED; + spin_unlock(&se_cmd->t_state_lock); + + return kref_get_unless_zero(&se_cmd->cmd_kref); +} + void core_tmr_abort_task( struct se_device *dev, struct se_tmr_req *tmr, @@ -133,26 +161,18 @@ void core_tmr_abort_task( printk("ABORT_TASK: Found referenced %s task_tag: %llu\n", se_cmd->se_tfo->get_fabric_name(), ref_tag); - spin_lock(&se_cmd->t_state_lock); - if (se_cmd->transport_state & CMD_T_COMPLETE) { - printk("ABORT_TASK: ref_tag: %llu already complete," - " skipping\n", ref_tag); - spin_unlock(&se_cmd->t_state_lock); + if (!__target_check_io_state(se_cmd)) { spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); goto out; } - se_cmd->transport_state |= CMD_T_ABORTED; - spin_unlock(&se_cmd->t_state_lock); - list_del_init(&se_cmd->se_cmd_list); - kref_get(&se_cmd->cmd_kref); spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); cancel_work_sync(&se_cmd->work); transport_wait_for_tasks(se_cmd); - target_put_sess_cmd(se_cmd); transport_cmd_finish_abort(se_cmd, true); + target_put_sess_cmd(se_cmd); printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for" " ref_tag: %llu\n", ref_tag); @@ -237,8 +257,10 @@ static void core_tmr_drain_state_list( struct list_head *preempt_and_abort_list) { LIST_HEAD(drain_task_list); + struct se_session *sess; struct se_cmd *cmd, *next; unsigned long flags; + int rc; /* * Complete outstanding commands with TASK_ABORTED SAM status. @@ -277,6 +299,17 @@ static void core_tmr_drain_state_list( if (prout_cmd == cmd)
[PATCH-v2 2/3] target: Fix LUN_RESET active TMR descriptor handling
From: Nicholas Bellinger This patch fixes a NULL pointer se_cmd->cmd_kref < 0 refcount bug during TMR LUN_RESET with active TMRs, triggered during se_cmd + se_tmr_req descriptor shutdown + release via core_tmr_drain_tmr_list(). To address this bug, go ahead and obtain a local kref_get_unless_zero(&se_cmd->cmd_kref) for active I/O to set CMD_T_ABORTED, and transport_wait_for_tasks() followed by the final target_put_sess_cmd() to drop the local ->cmd_kref. Also add two new checks within target_tmr_work() to avoid CMD_T_ABORTED -> TFO->queue_tm_rsp() callbacks ahead of invoking the backend -> fabric put in transport_cmd_check_stop_to_fabric(). For good measure, also change core_tmr_release_req() to use list_del_init() ahead of se_tmr_req memory free. Cc: Quinn Tran Cc: Himanshu Madhani Cc: Sagi Grimberg Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_tmr.c | 22 +- drivers/target/target_core_transport.c | 17 + 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 5f315b4..d4f30aa 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -68,7 +68,7 @@ void core_tmr_release_req(struct se_tmr_req *tmr) if (dev) { spin_lock_irqsave(&dev->se_tmr_lock, flags); - list_del(&tmr->tmr_list); + list_del_init(&tmr->tmr_list); spin_unlock_irqrestore(&dev->se_tmr_lock, flags); } @@ -193,9 +193,11 @@ static void core_tmr_drain_tmr_list( struct list_head *preempt_and_abort_list) { LIST_HEAD(drain_tmr_list); + struct se_session *sess; struct se_tmr_req *tmr_p, *tmr_pp; struct se_cmd *cmd; unsigned long flags; + bool rc; /* * Release all pending and outgoing TMRs aside from the received * LUN_RESET tmr.. @@ -221,17 +223,31 @@ static void core_tmr_drain_tmr_list( if (target_check_cdb_and_preempt(preempt_and_abort_list, cmd)) continue; + sess = cmd->se_sess; + if (WARN_ON_ONCE(!sess)) + continue; + + spin_lock(&sess->sess_cmd_lock); spin_lock(&cmd->t_state_lock); if (!(cmd->transport_state & CMD_T_ACTIVE)) { spin_unlock(&cmd->t_state_lock); + spin_unlock(&sess->sess_cmd_lock); continue; } if (cmd->t_state == TRANSPORT_ISTATE_PROCESSING) { spin_unlock(&cmd->t_state_lock); + spin_unlock(&sess->sess_cmd_lock); continue; } + cmd->transport_state |= CMD_T_ABORTED; spin_unlock(&cmd->t_state_lock); + rc = kref_get_unless_zero(&cmd->cmd_kref); + spin_unlock(&sess->sess_cmd_lock); + if (!rc) { + printk("LUN_RESET TMR: non-zero kref_get_unless_zero\n"); + continue; + } list_move_tail(&tmr_p->tmr_list, &drain_tmr_list); } spin_unlock_irqrestore(&dev->se_tmr_lock, flags); @@ -245,7 +261,11 @@ static void core_tmr_drain_tmr_list( (preempt_and_abort_list) ? "Preempt" : "", tmr_p, tmr_p->function, tmr_p->response, cmd->t_state); + cancel_work_sync(&cmd->work); + transport_wait_for_tasks(cmd); + transport_cmd_finish_abort(cmd, 1); + target_put_sess_cmd(cmd); } } diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 19500e3..6235067 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2905,8 +2905,17 @@ static void target_tmr_work(struct work_struct *work) struct se_cmd *cmd = container_of(work, struct se_cmd, work); struct se_device *dev = cmd->se_dev; struct se_tmr_req *tmr = cmd->se_tmr_req; + unsigned long flags; int ret; + spin_lock_irqsave(&cmd->t_state_lock, flags); + if (cmd->transport_state & CMD_T_ABORTED) { + tmr->response = TMR_FUNCTION_REJECTED; + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + goto check_stop; + } + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + switch (tmr->function) { case TMR_ABORT_TASK: core_tmr_abort_task(dev, tmr, cmd->se_sess); @@ -2939,9 +2948,17 @@ static void target_tmr_work(struct work_struct *work) break; } + spin_lock_irqsave(&cmd->t_state_lock, flags); + if (cmd->transport_state & CMD_T_ABORTED) { +
[PATCH-v2 3/3] target: Fix TAS handling for multi-session se_node_acls
From: Nicholas Bellinger This patch fixes a bug in TMR task aborted status (TAS) handling when multiple sessions are connected to the same target WWPN endpoint and se_node_acl descriptor, resulting in TASK_ABORTED status to not be generated for aborted se_cmds on the remote port. This is due to core_tmr_handle_tas_abort() incorrectly comparing se_node_acl instead of se_session, for which the multi-session case is expected to be sharing the same se_node_acl. Instead, go ahead and update core_tmr_handle_tas_abort() to compare tmr_sess + cmd->se_sess in order to determine if the LUN_RESET was received on a different I_T nexus, and TASK_ABORTED status response needs to be generated. Cc: Quinn Tran Cc: Himanshu Madhani Cc: Sagi Grimberg Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_tmr.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index d4f30aa..3eff563 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -76,7 +76,7 @@ void core_tmr_release_req(struct se_tmr_req *tmr) } static void core_tmr_handle_tas_abort( - struct se_node_acl *tmr_nacl, + struct se_session *tmr_sess, struct se_cmd *cmd, int tas) { @@ -84,7 +84,7 @@ static void core_tmr_handle_tas_abort( /* * TASK ABORTED status (TAS) bit support */ - if ((tmr_nacl && (tmr_nacl != cmd->se_sess->se_node_acl)) && tas) { + if ((tmr_sess && (tmr_sess != cmd->se_sess)) && tas) { remove = false; transport_send_task_abort(cmd); } @@ -111,7 +111,7 @@ static bool __target_check_io_state(struct se_cmd *se_cmd) { struct se_session *sess = se_cmd->se_sess; - assert_spin_locked(&se_session->sess_cmd_lock); + assert_spin_locked(&sess->sess_cmd_lock); WARN_ON_ONCE(!irqs_disabled()); /* * If command already reached CMD_T_COMPLETE state within @@ -272,7 +272,7 @@ static void core_tmr_drain_tmr_list( static void core_tmr_drain_state_list( struct se_device *dev, struct se_cmd *prout_cmd, - struct se_node_acl *tmr_nacl, + struct se_session *tmr_sess, int tas, struct list_head *preempt_and_abort_list) { @@ -364,7 +364,7 @@ static void core_tmr_drain_state_list( cancel_work_sync(&cmd->work); transport_wait_for_tasks(cmd); - core_tmr_handle_tas_abort(tmr_nacl, cmd, tas); + core_tmr_handle_tas_abort(tmr_sess, cmd, tas); target_put_sess_cmd(cmd); } } @@ -377,6 +377,7 @@ int core_tmr_lun_reset( { struct se_node_acl *tmr_nacl = NULL; struct se_portal_group *tmr_tpg = NULL; + struct se_session *tmr_sess = NULL; int tas; /* * TASK_ABORTED status bit, this is configurable via ConfigFS @@ -395,8 +396,9 @@ int core_tmr_lun_reset( * or struct se_device passthrough.. */ if (tmr && tmr->task_cmd && tmr->task_cmd->se_sess) { - tmr_nacl = tmr->task_cmd->se_sess->se_node_acl; - tmr_tpg = tmr->task_cmd->se_sess->se_tpg; + tmr_sess = tmr->task_cmd->se_sess; + tmr_nacl = tmr_sess->se_node_acl; + tmr_tpg = tmr_sess->se_tpg; if (tmr_nacl && tmr_tpg) { pr_debug("LUN_RESET: TMR caller fabric: %s" " initiator port %s\n", @@ -409,7 +411,7 @@ int core_tmr_lun_reset( dev->transport->name, tas); core_tmr_drain_tmr_list(dev, tmr, preempt_and_abort_list); - core_tmr_drain_state_list(dev, prout_cmd, tmr_nacl, tas, + core_tmr_drain_state_list(dev, prout_cmd, tmr_sess, tas, preempt_and_abort_list); /* -- 1.9.1 -- 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/2] target: Fix LUN_RESET active I/O handling for ACK_KREF
On Tue, 2016-01-12 at 16:20 +0100, Christoph Hellwig wrote: > > It also introduces SCF_ACK_KREF to determine when > > transport_cmd_finish_abort() needs to drop the second > > extra reference, ahead of calling target_put_sess_cmd() > > for the final kref_put(&se_cmd->cmd_kref). > > It would be really useful to have all drivers follow that > ACK KREF model instead of needing to deal with driver > differences everywhere.. > tcm_fc, usb-gadget, sbp-target and xen-scsiback are whom need to be converted. It should be easy enough to do for v4.6 code. > > Finally, move transport_put_cmd() release of SGL + > > TMR + extended CDB memory into target_free_cmd_mem() > > in order to avoid potential resource leaks in TMR > > ABORT_TASK + LUN_RESET code-paths. Also update > > target_release_cmd_kref() accordingly. > > Sounds like that should be a separate patch. > This should to stay with the original bug-fix for stable back-porting purposes, and it's been kept together with the original for now. It's time-consuming enough to back-port given the various upstream changes recently. > > +/* > > + * Called with se_session->sess_cmd_lock held with irq disabled > > + */ > > Please enforce this in the code instead of the comments, e.g. > > assert_spin_locked(&se_session->sess_cmd_lock); > WARN_ON_ONCE(!irqs_disabled()); > Done. > > +static bool __target_check_io_state(struct se_cmd *se_cmd) > > +{ > > + struct se_session *sess = se_cmd->se_sess; > > + > > + if (!sess) > > + return false; > > Given that you expect the session lock to be held this doesn't > make sense. > Dropped. > > + /* > > +* Obtain cmd_kref and move to list for shutdown processing > > +* if se_cmd->cmd_kref is still active, the command has not > > +* already reached CMD_T_COMPLETE > > +*/ > > This just explains what __target_check_io_state does, but no why. > I'd suggest to remove the comment as it doesn't add any value. How about the following for __target_check_io_state()..? /* * If command already reached CMD_T_COMPLETE state within * target_complete_cmd(), this se_cmd has been passed to * fabric driver and will not be aborted. * * Otherwise, obtain a local se_cmd->cmd_kref now for TMR * ABORT_TASK + LUN_RESET for CMD_T_ABORTED processing as * long as se_cmd->cmd_kref is still active unless zero. */ -- 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 2/2] target: Fix LUN_RESET active TMR descriptor handling
On Tue, 2016-01-12 at 16:25 +0100, Christoph Hellwig wrote: > > if (dev) { > > spin_lock_irqsave(&dev->se_tmr_lock, flags); > > - list_del(&tmr->tmr_list); > > + if (!list_empty(&tmr->tmr_list)) > > + list_del_init(&tmr->tmr_list); > > list_del_init on a empty list is fine. > Done. > > + sess = cmd->se_sess; > > + if (!sess) { > > + dump_stack(); > > + continue; > > + } > > Why not something like: > > if (WARN_ON_ONCE(!sess)) > continue; > > same for the previous patch. > Done. > > + spin_lock(&sess->sess_cmd_lock); > > + rc = kref_get_unless_zero(&cmd->cmd_kref); > > + spin_unlock(&sess->sess_cmd_lock); > > no need to take a lock around kref_get_unless_zero, it's not going to > help with anything. So for -v2, this lock is being obtained before ->t_state_lock above, to follow how __target_check_io_state() works with aborted I/O + LUN_RESET and explicit TMR_ABORT_TASK. It's been made consistent with the other cases for now, but depending on how the bug-fix for se_session shutdown plus remote port LUN_RESET works out, this may or may-not be able to go away for -v3. -- 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] mptlan: add checks for dma mapping errors
Hi Alexey, [auto build test WARNING on v4.4-rc8] [also build test WARNING on next-20160122] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Alexey-Khoroshilov/mptlan-add-checks-for-dma-mapping-errors/20160123-070633 config: x86_64-randconfig-s1-01230930 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All warnings (new ones prefixed by >>): In file included from include/uapi/linux/stddef.h:1:0, from include/linux/stddef.h:4, from include/uapi/linux/posix_types.h:4, from include/uapi/linux/types.h:13, from include/linux/types.h:5, from include/linux/list.h:4, from include/linux/module.h:9, from drivers/message/fusion/mptlan.h:55, from drivers/message/fusion/mptlan.c:55: drivers/message/fusion/mptlan.c: In function 'mpt_lan_sdu_send': drivers/message/fusion/mptlan.c:737:24: warning: passing argument 1 of 'dma_mapping_error' from incompatible pointer type [-Wincompatible-pointer-types] if (dma_mapping_error(mpt_dev->pcidev, dma)) { ^ include/linux/compiler.h:147:28: note: in definition of macro '__trace_if' if (__builtin_constant_p((cond)) ? !!(cond) : \ ^ >> drivers/message/fusion/mptlan.c:737:2: note: in expansion of macro 'if' if (dma_mapping_error(mpt_dev->pcidev, dma)) { ^ In file included from arch/x86/include/asm/dma-mapping.h:49:0, from include/linux/dma-mapping.h:87, from include/linux/skbuff.h:34, from include/linux/if_ether.h:23, from include/uapi/linux/ethtool.h:17, from include/linux/ethtool.h:16, from include/linux/netdevice.h:42, from drivers/message/fusion/mptlan.h:58, from drivers/message/fusion/mptlan.c:55: include/asm-generic/dma-mapping-common.h:316:19: note: expected 'struct device *' but argument is of type 'struct pci_dev *' static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr) ^ In file included from include/uapi/linux/stddef.h:1:0, from include/linux/stddef.h:4, from include/uapi/linux/posix_types.h:4, from include/uapi/linux/types.h:13, from include/linux/types.h:5, from include/linux/list.h:4, from include/linux/module.h:9, from drivers/message/fusion/mptlan.h:55, from drivers/message/fusion/mptlan.c:55: drivers/message/fusion/mptlan.c:737:24: warning: passing argument 1 of 'dma_mapping_error' from incompatible pointer type [-Wincompatible-pointer-types] if (dma_mapping_error(mpt_dev->pcidev, dma)) { ^ include/linux/compiler.h:147:40: note: in definition of macro '__trace_if' if (__builtin_constant_p((cond)) ? !!(cond) : \ ^ >> drivers/message/fusion/mptlan.c:737:2: note: in expansion of macro 'if' if (dma_mapping_error(mpt_dev->pcidev, dma)) { ^ In file included from arch/x86/include/asm/dma-mapping.h:49:0, from include/linux/dma-mapping.h:87, from include/linux/skbuff.h:34, from include/linux/if_ether.h:23, from include/uapi/linux/ethtool.h:17, from include/linux/ethtool.h:16, from include/linux/netdevice.h:42, from drivers/message/fusion/mptlan.h:58, from drivers/message/fusion/mptlan.c:55: include/asm-generic/dma-mapping-common.h:316:19: note: expected 'struct device *' but argument is of type 'struct pci_dev *' static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr) ^ In file included from include/uapi/linux/stddef.h:1:0, from include/linux/stddef.h:4, from include/uapi/linux/posix_types.h:4, from include/uapi/linux/types.h:13, from include/linux/types.h:5, from include/linux/list.h:4, from include/linux/module.h:9, from drivers/message/fusion/mptlan.h:55, from drivers/message/fusion/mptlan.c:55: drivers/message/fusion/mptlan.c:737:24: warning: passing argument 1 of 'dma_mapping_error' from incompatible pointer type [-Wincompatible-pointer-types] if
RE: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET
Hello Tomas, > -Original Message- > From: Tomas Henzl [mailto:the...@redhat.com] > Sent: Friday, January 22, 2016 5:15 AM > To: Raghava Aditya Renukunta; james.bottom...@hansenpartnership.com; > martin.peter...@oracle.com; linux-scsi@vger.kernel.org > Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc- > sierra.com; Scott Benesh; jthumsh...@suse.de; shane.seym...@hpe.com; > zzzDavid Carroll > Subject: Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET > > On 20.1.2016 21:32, Raghava Aditya Renukunta wrote: > > > >> -Original Message- > >> From: Tomas Henzl [mailto:the...@redhat.com] > >> Sent: Tuesday, January 19, 2016 8:14 AM > >> To: Raghava Aditya Renukunta; > james.bottom...@hansenpartnership.com; > >> martin.peter...@oracle.com; linux-scsi@vger.kernel.org > >> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc- > >> sierra.com; Scott Benesh; jthumsh...@suse.de; > shane.seym...@hpe.com; > >> David Carroll > >> Subject: Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET > >> > >> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote: > >>> From: Raghava Aditya Renukunta > > >>> > >>> while driver removal is in progress or PCI shutdown is invoked, driver > >>> kills AIF aacraid thread, but IOCTL requests from the management tools > >>> re-start AIF thread leading to IOP_RESET. > >>> > >>> Fixed by setting adapter_shutdown flag when PCI shutdown is invoked. > >>> > >>> Changes in V2: > >>> Set adapter_shutdown flag before shutdown command is sent to \ > >>> controller > >>> > >>> Changes in V3: > >>> Call aac_send_shut_shutdown first thing in __aac_shutdown > >>> Convert adapter_shutdown to atomic_t variable to prevent \ > >>> SMP coherency issues(race conditions) > >> Hi, > >> I don't think that an atomic variable can change it, imagine that > >> thread just passed your test in aac_cfg_ioctl and another thread > >> enter a bit later the adapter_shutdown so both - an I/O and your > shutdown > >> code > >> will run in parallel. > >> Also you need to fix your compat_ioctl path too. > >> > >> --tm > > Hello Tomas, > > Yes that would not solve this problem. > > I can think of 2 solutions > > > > 1.Place a delay after setting adapter_shutdown and before sending the > actual > > shutdown command in aac_send_shutdown. This way any impending > commands will be > > processed before the adapter actually receives the shutdown command. > Since management > > commands are sync only , shutdown command will be sent last. This > solution uses an > > estimation of the delay > > > > 2.Since commands are sync , place a check in aac_fib_send to block > > commands once adapter_shutdown is set(only shutdown command will > be sent thru) > > This option looks better but I guess you still can find a tiny race window. > What do you think about a mutual exclusive access using a mutex, do you > think this could work? > diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c > index 54195a117f..b9505c58db 100644 > --- a/drivers/scsi/aacraid/commctrl.c > +++ b/drivers/scsi/aacraid/commctrl.c > @@ -858,6 +858,8 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void > __user *arg) > /* >* HBA gets first crack >*/ > + if (dev->adapter_shutdown) > + return -EACCES; > > status = aac_dev_ioctl(dev, cmd, arg); > if (status != -ENOTTY) > diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c > index 0e954e37f0..02535c07a4 100644 > --- a/drivers/scsi/aacraid/comminit.c > +++ b/drivers/scsi/aacraid/comminit.c > @@ -212,6 +212,10 @@ int aac_send_shutdown(struct aac_dev * dev) > return -ENOMEM; > aac_fib_init(fibctx); > > + mutex_lock(&aac_mutex); > + dev->adapter_shutdown = 1; > + mutex_unlock(&aac_mutex); > + The intention here is to have mutually exclusive access to adapter_shutdown, or aac_do_Ioctl which has a check for adapter_shutdown so either one can be command is sent at one time if I am not mistaken?. Yes this solution will work. I will make the necessary changes. Thank you. Regards, Raghava Aditya > cmd = (struct aac_close *) fib_data(fibctx); > > cmd->command = cpu_to_le32(VM_CloseAll); > @@ -229,7 +233,6 @@ int aac_send_shutdown(struct aac_dev * dev) > /* FIB should be freed only after getting the response from the F/W > */ > if (status != -ERESTARTSYS) > aac_fib_free(fibctx); > - dev->adapter_shutdown = 1; > if ((dev->pdev->device == PMC_DEVICE_S7 || >dev->pdev->device == PMC_DEVICE_S8 || >dev->pdev->device == PMC_DEVICE_S9) && > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c > index 6944560e22..a87880ab32 100644 > --- a/drivers/scsi/aacraid/linit.c > +++ b/drivers/scsi/aacraid/linit.c > @@ -706,8 +706,9 @@ static long aac_cfg_ioctl(struct file *file, > int ret; > struct aac_dev *aac; > aac = (struct aac_dev *)file
RE: [PATCH V3 8/9] aacraid: Fix character device re-initialization
> -Original Message- > From: Tomas Henzl [mailto:the...@redhat.com] > Sent: Friday, January 22, 2016 5:08 AM > To: Raghava Aditya Renukunta; james.bottom...@hansenpartnership.com; > martin.peter...@oracle.com; linux-scsi@vger.kernel.org > Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc- > sierra.com; Scott Benesh; jthumsh...@suse.de; shane.seym...@hpe.com; > zzzDavid Carroll > Subject: Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization > > On 20.1.2016 21:43, Raghava Aditya Renukunta wrote: > > Hello Tomas, > > > >> -Original Message- > >> From: Tomas Henzl [mailto:the...@redhat.com] > >> Sent: Wednesday, January 20, 2016 5:41 AM > >> To: Raghava Aditya Renukunta; > james.bottom...@hansenpartnership.com; > >> martin.peter...@oracle.com; linux-scsi@vger.kernel.org > >> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc- > >> sierra.com; Scott Benesh; jthumsh...@suse.de; > shane.seym...@hpe.com; > >> zzzDavid Carroll > >> Subject: Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization > >> > >> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote: > >>> From: Raghava Aditya Renukunta > > >>> > >>> During EEH PCI hotplug activity kernel unloads and loads the driver, > >>> causing character device to be unregistered(aac_remove_one).When > the > >>> driver is loaded back using aac_probe_one the character device needs > >>> to be registered again for the AIF management tools to work. > >>> > >>> Fixed by adding code to register character device in aac_probe_one if > >>> it is unregistered in aac_remove_one. > >>> > >>> Changes in V2: > >>> Added macros to track character device state > >>> > >>> Changes in V3: > >>> None > >>> > >>> Signed-off-by: Raghava Aditya Renukunta > >> > >>> Reviewed-by: Shane Seymour > >> Hi Raghava, > >> when aacraid is loaded (modprobe) without an controller attached to the > >> system > >> the driver loads and creates the character device. Later when you hotplug > a > >> device and remove again we see the driver loaded but now without the > >> char device. I'd prefer consistency here - either create the char device > >> when the first controller is probed (preferred) or do not remove it > >> until the driver exits. > >> This is not a nack, just a wish that you changed it in next series. > >> > >> --tm > > > > Yes I will make the necessary changes so that character device is created > when > > The controller is probed, and when the driver is removed > (aac_remove_one),delete > > the character device. I will keep the character device during resume and > suspend. > > > > Do you want to do this in the next version of the patches or the next series > of patches after this one is > > Accepted. ? > > sure, next series is fine, as I wrote already Will do , Thank you Tomas. > > > > Regards, > > Raghava Aditya > > > > > >>> --- > >>> drivers/scsi/aacraid/aacraid.h | 7 +++ > >>> drivers/scsi/aacraid/linit.c | 21 ++--- > >>> 2 files changed, 21 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/drivers/scsi/aacraid/aacraid.h > >>> b/drivers/scsi/aacraid/aacraid.h > >>> index 3473668..4b669ef 100644 > >>> --- a/drivers/scsi/aacraid/aacraid.h > >>> +++ b/drivers/scsi/aacraid/aacraid.h > >>> @@ -94,6 +94,13 @@ enum { > >>> #define aac_phys_to_logical(x) ((x)+1) > >>> #define aac_logical_to_phys(x) ((x)?(x)-1:0) > >>> > >>> +/* > >>> + * These macros are for keeping track of > >>> + * character device state. > >>> + */ > >>> +#define AAC_CHARDEV_UNREGISTERED (-1) > >>> +#define AAC_CHARDEV_NEEDS_REINIT (-2) > >>> + > >>> /* #define AAC_DETAILED_STATUS_INFO */ > >>> > >>> struct diskparm > >>> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c > >>> index 27b3fcd..057c07c 100644 > >>> --- a/drivers/scsi/aacraid/linit.c > >>> +++ b/drivers/scsi/aacraid/linit.c > >>> @@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION); > >>> > >>> static DEFINE_MUTEX(aac_mutex); > >>> static LIST_HEAD(aac_devices); > >>> -static int aac_cfg_major = -1; > >>> +static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED; > >>> char aac_driver_version[] = AAC_DRIVER_FULL_VERSION; > >>> > >>> /* > >>> @@ -1125,6 +1125,13 @@ static void __aac_shutdown(struct aac_dev * > >> aac) > >>> else if (aac->max_msix > 1) > >>> pci_disable_msix(aac->pdev); > >>> } > >>> +static void aac_init_char(void) > >>> +{ > >>> + aac_cfg_major = register_chrdev(0, "aac", &aac_cfg_fops); > >>> + if (aac_cfg_major < 0) { > >>> + pr_err("aacraid: unable to register \"aac\" device.\n"); > >>> + } > >>> +} > >>> > >>> static int aac_probe_one(struct pci_dev *pdev, const struct > pci_device_id > >> *id) > >>> { > >>> @@ -1182,6 +1189,9 @@ static int aac_probe_one(struct pci_dev *pdev, > >> const struct pci_device_id *id) > >>> shost->max_cmd_len = 16; > >>> shost->use_cmd_list = 1; > >>> > >>> + if (aac_cfg_major == AAC_CHARDEV_NEEDS_REINIT) > >>> + aac_init_char(); > >>> + >
RE: [PATCH] drivers/scsi/emcctd: drivers/scsi/emcctd: Client driver implementation for EMC-Symmetrix GuestOS emulated Cut-Through Device
Hello Thumshirn. Thanks for taking out time to review the patch. I appreciate that. Please find my comments inlined. > -Original Message- > From: Johannes Thumshirn [mailto:jthumsh...@suse.de] > Sent: Tuesday, January 19, 2016 7:20 PM > To: Singhal, Maneesh > Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; > jbottom...@odin.com; martin.peter...@oracle.com; linux- > a...@vger.kernel.org > Subject: Re: [PATCH] drivers/scsi/emcctd: drivers/scsi/emcctd: Client > driver implementation for EMC-Symmetrix GuestOS emulated Cut- > Through Device > > On Tue, Jan 19, 2016 at 11:58:06AM +, Singhal, Maneesh wrote: > > Hello, > > Kindly review the following patch for the following driver to be > added in SCSI subsystem - > > > > Regards > > Maneesh > > > > Hi Maneesh, > > Fery first round of review. No real functionallity yet just a bit on > readablility. > > I'll do a more in depth review later. > > > > From f3c4b836d6f130b1d7ded618002c8164f8f4a06d Mon Sep 17 > 00:00:00 2001 > > From: "maneesh.singhal" > > Date: Tue, 19 Jan 2016 06:39:35 -0500 > > Subject: [PATCH] [PATCH] drivers/scsi/emcctd: Client driver > implementation for > > EMC-Symmetrix GuestOS emulated Cut-Through Device. > > > > The patch is a driver implementation EMC-Symmetrix GuestOS > emulated Cut-Through > > Device. The Cut-Through Device PCI emulation is implemented for > GuestOS > > environments in the HyperMax OS. GuestOS environments allows > loading of > > any x86 compliant operating systems like Linux/FreeBSD etc. > > > > The client driver is a SCSI HBA implementation which interfaces with > SCSI > > midlayer in the north-bound interfaces and connects with the > emulated PCI device > > on the south side. > > > > The PCI vendor ID:product ID for emulated Cut-Through Device is > 0x1120:0x1B00. > > > > Signed-off-by: maneesh.singhal > > --- > > Documentation/scsi/emcctd.txt | 57 + > > MAINTAINERS |9 + > > drivers/scsi/Kconfig|1 + > > drivers/scsi/Makefile |1 + > > drivers/scsi/emcctd/Kconfig |7 + > > drivers/scsi/emcctd/Makefile|1 + > > drivers/scsi/emcctd/README | 10 + > > drivers/scsi/emcctd/emc_ctd_interface.h | 386 + > > drivers/scsi/emcctd/emcctd.c| 2840 > +++ > > drivers/scsi/emcctd/emcctd.h| 232 +++ > > 10 files changed, 3544 insertions(+) > > create mode 100644 Documentation/scsi/emcctd.txt > > create mode 100644 drivers/scsi/emcctd/Kconfig > > create mode 100644 drivers/scsi/emcctd/Makefile > > create mode 100644 drivers/scsi/emcctd/README > > create mode 100644 drivers/scsi/emcctd/emc_ctd_interface.h > > create mode 100644 drivers/scsi/emcctd/emcctd.c > > create mode 100644 drivers/scsi/emcctd/emcctd.h > > > > diff --git a/Documentation/scsi/emcctd.txt > b/Documentation/scsi/emcctd.txt > > new file mode 100644 > > index 000..bcafc87 > > --- /dev/null > > +++ b/Documentation/scsi/emcctd.txt > > @@ -0,0 +1,56 @@ > > +This file contains brief information about the EMC Cut-Through > Driver (emcctd). > > +The driver is currently maintained by Singhal, Maneesh > (maneesh.sing...@emc.com) > > + > > +Last modified: Mon Jan 18 2016 by Maneesh Singhal > > + > > +BASICS > > + > > +Its a client driver implementation for EMC-Symmetrix GuestOS > emulated > > +Cut-Through Device. The Cut-Through Device PCI emulation is > implemented for > > +GuestOS environments in the HyperMax OS. GuestOS > environments allows loading of > > +any x86 compliant operating systems like Linux/FreeBSD etc. > > + > > +The client driver is a SCSI HBA implementation which interfaces with > SCSI > > +midlayer in the north-bound interfaces and connects with the > emulated PCI device > > +on the south side. > > + > > +The PCI vendor ID:product ID for emulated Cut-Through Device is > 0x1120:0x1B00. > > + > > +VERSIONING > > + > > +The Version of the driver is maintained as 2.0.0.X, where 2 refers to > the CTD > > +protocol in use, and X refers to the ongoing version of the driver. > > + > > + > > +SYSFS SUPPORT > > + > > +The driver creates the directory /sys/module/emcctd and > populates it with > > +version file and a directory for various parameters as described in > MODULE > > +PARAMETERS section. > > + > > +PROCFS SUPPORT > > + > > +The driver creates the directory /proc/emc and creates files > emcctd_stats_x > > +where 'x' refers to the PCI emulation number this client driver > connected to. > > +These files cotains WWN information and IO statistics for the > particular PCI > > +emulation. > > + > > +MODULE PARAMETERS > > + > > +The supported parameters which could add debuggability or change > the runtime > > +behavior of the driver are as following: > > + > > +ctd_debug=0 | 1Enable driver debug messages(0=off, > 1=on) > > + > > +max_luns=x
RE: [PATCH] drivers/scsi/emcctd: drivers/scsi/emcctd: Client driver implementation for EMC-Symmetrix GuestOS emulated Cut-Through Device
Thanks for your time. My replies inlined... > -Original Message- > From: Johannes Thumshirn [mailto:jthumsh...@suse.de] > Sent: Tuesday, January 19, 2016 9:34 PM > To: Singhal, Maneesh > Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; > jbottom...@odin.com; martin.peter...@oracle.com; linux- > a...@vger.kernel.org > Subject: Re: [PATCH] drivers/scsi/emcctd: drivers/scsi/emcctd: Client > driver implementation for EMC-Symmetrix GuestOS emulated Cut- > Through Device > > On Tue, Jan 19, 2016 at 11:58:06AM +, Singhal, Maneesh wrote: > > Hello, > > Kindly review the following patch for the following driver to be > added in SCSI subsystem - > > > > Regards > > Maneesh > > > > > > From f3c4b836d6f130b1d7ded618002c8164f8f4a06d Mon Sep 17 > 00:00:00 2001 > > From: "maneesh.singhal" > > Date: Tue, 19 Jan 2016 06:39:35 -0500 > > Subject: [PATCH] [PATCH] drivers/scsi/emcctd: Client driver > implementation for > > EMC-Symmetrix GuestOS emulated Cut-Through Device. > > > > The patch is a driver implementation EMC-Symmetrix GuestOS > emulated Cut-Through > > Device. The Cut-Through Device PCI emulation is implemented for > GuestOS > > environments in the HyperMax OS. GuestOS environments allows > loading of > > any x86 compliant operating systems like Linux/FreeBSD etc. > > > > The client driver is a SCSI HBA implementation which interfaces with > SCSI > > midlayer in the north-bound interfaces and connects with the > emulated PCI device > > on the south side. > > > > The PCI vendor ID:product ID for emulated Cut-Through Device is > 0x1120:0x1B00. > > > > Signed-off-by: maneesh.singhal > > --- > > Documentation/scsi/emcctd.txt | 57 + > > MAINTAINERS |9 + > > drivers/scsi/Kconfig|1 + > > drivers/scsi/Makefile |1 + > > drivers/scsi/emcctd/Kconfig |7 + > > drivers/scsi/emcctd/Makefile|1 + > > drivers/scsi/emcctd/README | 10 + > > drivers/scsi/emcctd/emc_ctd_interface.h | 386 + > > drivers/scsi/emcctd/emcctd.c| 2840 > +++ > > drivers/scsi/emcctd/emcctd.h| 232 +++ > > 10 files changed, 3544 insertions(+) > > create mode 100644 Documentation/scsi/emcctd.txt > > create mode 100644 drivers/scsi/emcctd/Kconfig > > create mode 100644 drivers/scsi/emcctd/Makefile > > create mode 100644 drivers/scsi/emcctd/README > > create mode 100644 drivers/scsi/emcctd/emc_ctd_interface.h > > create mode 100644 drivers/scsi/emcctd/emcctd.c > > create mode 100644 drivers/scsi/emcctd/emcctd.h > > > > diff --git a/Documentation/scsi/emcctd.txt > b/Documentation/scsi/emcctd.txt > > new file mode 100644 > > index 000..bcafc87 > > --- /dev/null > > +++ b/Documentation/scsi/emcctd.txt > > @@ -0,0 +1,56 @@ > > +This file contains brief information about the EMC Cut-Through > Driver (emcctd). > > +The driver is currently maintained by Singhal, Maneesh > (maneesh.sing...@emc.com) > > + > > +Last modified: Mon Jan 18 2016 by Maneesh Singhal > > + > > +BASICS > > + > > +Its a client driver implementation for EMC-Symmetrix GuestOS > emulated > > +Cut-Through Device. The Cut-Through Device PCI emulation is > implemented for > > +GuestOS environments in the HyperMax OS. GuestOS > environments allows loading of > > +any x86 compliant operating systems like Linux/FreeBSD etc. > > + > > +The client driver is a SCSI HBA implementation which interfaces with > SCSI > > +midlayer in the north-bound interfaces and connects with the > emulated PCI device > > +on the south side. > > + > > +The PCI vendor ID:product ID for emulated Cut-Through Device is > 0x1120:0x1B00. > > + > > +VERSIONING > > + > > +The Version of the driver is maintained as 2.0.0.X, where 2 refers to > the CTD > > +protocol in use, and X refers to the ongoing version of the driver. > > + > > + > > +SYSFS SUPPORT > > + > > +The driver creates the directory /sys/module/emcctd and > populates it with > > +version file and a directory for various parameters as described in > MODULE > > +PARAMETERS section. > > + > > +PROCFS SUPPORT > > + > > +The driver creates the directory /proc/emc and creates files > emcctd_stats_x > > +where 'x' refers to the PCI emulation number this client driver > connected to. > > +These files cotains WWN information and IO statistics for the > particular PCI > > +emulation. > > + > > +MODULE PARAMETERS > > + > > +The supported parameters which could add debuggability or change > the runtime > > +behavior of the driver are as following: > > + > > +ctd_debug=0 | 1Enable driver debug messages(0=off, > 1=on) > > + > > +max_luns=xxSpecify the maximum number of LUN's > per > > + host(default=16384) > > + > > +cmd_per_lun=xx Specify the maximum commands per > lun(default=16) > > + > > +DEBUGGING HINTS > > + >
RE: [PATCH] drivers/scsi/emcctd: drivers/scsi/emcctd: Client driver implementation for EMC-Symmetrix GuestOS emulated Cut-Through Device
Hello Greg, Thanks for taking out time to review the patch. Please find my replies inlined... Will post the next patch for modifications soon. > -Original Message- > From: Greg KH [mailto:g...@kroah.com] > Sent: Tuesday, January 19, 2016 11:42 PM > To: Singhal, Maneesh > Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; > jbottom...@odin.com; martin.peter...@oracle.com; linux- > a...@vger.kernel.org > Subject: Re: [PATCH] drivers/scsi/emcctd: drivers/scsi/emcctd: Client > driver implementation for EMC-Symmetrix GuestOS emulated Cut- > Through Device > > On Tue, Jan 19, 2016 at 11:58:06AM +, Singhal, Maneesh wrote: > > Hello, > > Kindly review the following patch for the following driver to be > added > > in SCSI subsystem - > > > > Regards > > Maneesh > > > > -- > > -- > > >From f3c4b836d6f130b1d7ded618002c8164f8f4a06d Mon Sep 17 > 00:00:00 > > >2001 > > From: "maneesh.singhal" > > Date: Tue, 19 Jan 2016 06:39:35 -0500 > > Subject: [PATCH] [PATCH] drivers/scsi/emcctd: Client driver > > implementation for EMC-Symmetrix GuestOS emulated Cut- > Through Device. > > > > The patch is a driver implementation EMC-Symmetrix GuestOS > emulated > > Cut-Through Device. The Cut-Through Device PCI emulation is > > implemented for GuestOS environments in the HyperMax OS. > GuestOS > > environments allows loading of any x86 compliant operating systems > like Linux/FreeBSD etc. > > > > The client driver is a SCSI HBA implementation which interfaces with > > SCSI midlayer in the north-bound interfaces and connects with the > > emulated PCI device on the south side. > > > > The PCI vendor ID:product ID for emulated Cut-Through Device is > 0x1120:0x1B00. > > > > Signed-off-by: maneesh.singhal > > --- > > Documentation/scsi/emcctd.txt | 57 + > > MAINTAINERS |9 + > > drivers/scsi/Kconfig|1 + > > drivers/scsi/Makefile |1 + > > drivers/scsi/emcctd/Kconfig |7 + > > drivers/scsi/emcctd/Makefile|1 + > > drivers/scsi/emcctd/README | 10 + > > drivers/scsi/emcctd/emc_ctd_interface.h | 386 + > > drivers/scsi/emcctd/emcctd.c| 2840 > +++ > > drivers/scsi/emcctd/emcctd.h| 232 +++ > > 10 files changed, 3544 insertions(+) > > create mode 100644 Documentation/scsi/emcctd.txt create mode > 100644 > > drivers/scsi/emcctd/Kconfig create mode 100644 > > drivers/scsi/emcctd/Makefile create mode 100644 > > drivers/scsi/emcctd/README create mode 100644 > > drivers/scsi/emcctd/emc_ctd_interface.h > > create mode 100644 drivers/scsi/emcctd/emcctd.c create mode > 100644 > > drivers/scsi/emcctd/emcctd.h > > > > diff --git a/Documentation/scsi/emcctd.txt > > b/Documentation/scsi/emcctd.txt new file mode 100644 index > > 000..bcafc87 > > --- /dev/null > > +++ b/Documentation/scsi/emcctd.txt > > @@ -0,0 +1,56 @@ > > +This file contains brief information about the EMC Cut-Through > Driver (emcctd). > > +The driver is currently maintained by Singhal, Maneesh > > +(maneesh.sing...@emc.com) > > + > > +Last modified: Mon Jan 18 2016 by Maneesh Singhal > > + > > +BASICS > > + > > +Its a client driver implementation for EMC-Symmetrix GuestOS > emulated > > +Cut-Through Device. The Cut-Through Device PCI emulation is > > +implemented for GuestOS environments in the HyperMax OS. > GuestOS > > +environments allows loading of any x86 compliant operating > systems like Linux/FreeBSD etc. > > + > > +The client driver is a SCSI HBA implementation which interfaces with > > +SCSI midlayer in the north-bound interfaces and connects with the > > +emulated PCI device on the south side. > > + > > +The PCI vendor ID:product ID for emulated Cut-Through Device is > 0x1120:0x1B00. > > + > > +VERSIONING > > + > > +The Version of the driver is maintained as 2.0.0.X, where 2 refers to > > +the CTD protocol in use, and X refers to the ongoing version of the > driver. > > + > > + > > +SYSFS SUPPORT > > + > > +The driver creates the directory /sys/module/emcctd and > populates it > > +with version file and a directory for various parameters as described > > +in MODULE PARAMETERS section. > > + > > +PROCFS SUPPORT > > + > > +The driver creates the directory /proc/emc and creates files > > +emcctd_stats_x where 'x' refers to the PCI emulation number this > client driver connected to. > > +These files cotains WWN information and IO statistics for the > > +particular PCI emulation. > > No, no driver should be adding proc files, please use sysfs or debugfs > for debugging things. [MS>] Sure. > > > +MODULE PARAMETERS > > No driver should be using module parameters anymore, again, please > use the correct interfaces. > [MS>] Yes got that > > + > > +The supported parameters which could add debuggability or change > the > > +runtime behavior of the driver are as following: >