ATTN::::: PROPOSAL

[ST CHANG]

China Steel Corporation (CSAC), is in urgent need of a reputable company,/firm
 or individual to serve as our financial coordinator in Canada, America, Europe,
 Uk. It's a part time job and pays well. If you are interested in working with 
us
 please reply: stchan...@yahoo.com.hk

Thank you very much.
S T CHANG
--
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: I/O path cleanup

[Bart Van Assche]

On 09/07/14 18:31, Christoph Hellwig wrote:

This series cleans up a couple of lose ends I noticed during the scsi-mq
work, but which weren't important enough to address during the last cycle.


Hello Christoph,

Is there perhaps a public git tree available with this patch series ? I 
have tried to apply these patches on top of v3.17-rc4 but unfortunately 
without success:

$ git checkout v3.17-rc4 -b scsi-core-cleanup
$ for p in \[PATCH\ *; do echo  $p && git am -C1 "$p" || break; done
 [PATCH 1_8] scsi: don't use scsi_next_command in 
scsi_reset_provider - Christoph Hellwig  - 2014-09-07 1831.eml

Applying: scsi: don't use scsi_next_command in scsi_reset_provider
 [PATCH 2_8] scsi: remove scsi_next_command - Christoph Hellwig 
 - 2014-09-07 1831.eml

Applying: scsi: remove scsi_next_command
 [PATCH 3_8] scsi: clean up S_G table freeing - Christoph Hellwig 
 - 2014-09-07 1831.eml

Applying: scsi: clean up S/G table freeing
/home/bart/software/linux-kernel/.git/rebase-apply/patch:138: trailing 
whitespace.


Context reduced to (1/1) to apply fragment at 640
Context reduced to (2/2) to apply fragment at 646
warning: 1 line adds whitespace errors.
 [PATCH 4_8] scsi: stop passing a gfp_mask argument down the command 
setup path - Christoph Hellwig  - 2014-09-07 1831.eml

Applying: scsi: stop passing a gfp_mask argument down the command setup path
 [PATCH 5_8] scsi: move scsi_dispatch_cmd to scsi_lib.c - Christoph 
Hellwig  - 2014-09-07 1831.eml

Applying: scsi: move scsi_dispatch_cmd to scsi_lib.c
error: patch failed: drivers/scsi/scsi.c:626
error: drivers/scsi/scsi.c: patch does not apply
Patch failed at 0001 scsi: move scsi_dispatch_cmd to scsi_lib.c
The copy of the patch that failed is found in:
   /home/bart/software/linux-kernel/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Bart.

--
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] bnx2i: Make boot_nic entry visible in the sysfs session objects

[Maurizio Lombardi]

Hi Cristoph, Jens,

On 05/19/2014 01:32 PM, vikas.chaudh...@qlogic.com wrote:
> From: Tej Parkash 
> 
> Signed-off-by: Tej Parkash 
> Signed-off-by: Vikas Chaudhary 
> ---
>  drivers/scsi/bnx2i/bnx2i_iscsi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
> b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> index 166543f..cabc8c1 100644
> --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
> +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> @@ -2234,6 +2234,9 @@ static umode_t bnx2i_attr_is_visible(int param_type, 
> int param)
>   case ISCSI_PARAM_TGT_RESET_TMO:
>   case ISCSI_PARAM_IFACE_NAME:
>   case ISCSI_PARAM_INITIATOR_NAME:
> + case ISCSI_PARAM_BOOT_ROOT:
> + case ISCSI_PARAM_BOOT_NIC:
> + case ISCSI_PARAM_BOOT_TARGET:
>   return S_IRUGO;
>   default:
>   return 0;
> 

Can you merge this patch? It has been ACKed already.

Thanks,
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] scsi_debug: deadlock between completions and surprise module removal

[Bart Van Assche]

On 09/06/14 16:40, Douglas Gilbert wrote:

On 14-09-05 11:25 AM, Bart Van Assche wrote:

An LLD must call scsi_remove_host() directly or indirectly from the
module
cleanup path. scsi_remove_host() triggers a call to
blk_cleanup_queue(). That
last function sets the flag QUEUE_FLAG_DYING which prevents that new
I/O is
queued and waits until previously queued requests have finished before
returning.


And they do call scsi_remove_host(). But they do that toward
the end of their clean-up. The problem that I observed has
already happened before that.

IOW I think the QUEUE_FLAG_DYING state needs to be set and
acknowledged as the first order of business by the code
that implements 'rmmod LLD'.


Hello Doug,

In the scsi_debug driver scsi_remove_host() is called from inside the 
sdebug_driver_remove() callback function. Unless I have missed something 
it is not guaranteed that that callback function is invoked before 
unloading of the scsi_debug driver has finished. I think most of the 
code in sdebug_driver_remove() should be moved to sdebug_remove_adapter().


Bart.

--
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: [GIT PULL] target updates for v3.17-rc3

[Stephen Rothwell]

Hi Nicholas,

On Sun, 31 Aug 2014 17:51:07 -0700 Linus Torvalds 
 wrote:
>
> On Sun, Aug 31, 2014 at 11:59 AM, Nicholas A. Bellinger
>  wrote:
> >
> > Note that these patches where originally intended for -rc1, but missed
> > the merge window.  They are mostly iser-target related bug-fixes, along
> > with a few other very minor cleanups.
> 
> So this pull request was strictly speaking too late for rc3, but I
> went "what the hell, it's small" and pulled it anyway.
> 
> And then I get this:
> 
>   drivers/target/target_core_transport.c: In function
> ‘transport_dump_vpd_ident_type’:
>   drivers/target/target_core_transport.c:956:3: warning: passing
> argument 1 of ‘strlen’ makes pointer from integer without a cast
> [enabled by default]
>  len = strlen(len);
>  ^
>   In file included from include/linux/bitmap.h:8:0,
>from include/linux/cpumask.h:11,
>from ./arch/x86/include/asm/cpumask.h:4,
>from ./arch/x86/include/asm/msr.h:10,
>from ./arch/x86/include/asm/processor.h:20,
>from ./arch/x86/include/asm/archrandom.h:26,
>from include/linux/random.h:81,
>from include/linux/net.h:22,
>from drivers/target/target_core_transport.c:26:
>   include/linux/string.h:80:24: note: expected ‘const char *’ but
> argument is of type ‘int’
>extern __kernel_size_t strlen(const char *);
>   ^
> 
> and I just go "Yeah, that broken crap can wait until 3.18 after all".
> 
> So it got unpulled.
> 
> That "strlen(len)" should clearly be a "strlen(buf)" in that commit
> 6cfa853ceee4, but equally clearly this pull request was pure and utter
> garbage, and that "cleanup" commit was shit that nobody had ever even
> bothered to compile.

I noticed this in linux-next today as well (in the target-update
tree).  Could I have that cleaned up, please?

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au


signature.asc
Description: PGP signature

[patch] xen-scsifront: use GFP_ATOMIC under spin_lock

[Dan Carpenter]

This function is only called with a spin_lock held and IRQs disabled.
The allocation is not allowed to sleep and NOIO is not sufficient, it
has to be ATOMIC.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index 0aceb70..7e88659 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -359,7 +359,7 @@ static int map_data_for_request(struct vscsifrnt_info *info,
}
seg_grants = vscsiif_grants_sg(data_grants);
shadow->sg = kcalloc(data_grants,
-   sizeof(struct scsiif_request_segment), GFP_NOIO);
+   sizeof(struct scsiif_request_segment), GFP_ATOMIC);
if (!shadow->sg)
return -ENOMEM;
}
--
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] xen-scsiback: clean up a type issue in scsiback_make_tpg()

[Dan Carpenter]

This code was confusing because we had an unsigned long and then we
compared it to UINT_MAX and then we stored it in a u16.  How many bytes
is this supposed to have: 2, 4 or 16???

I've made it a u16 throughout.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 7b565632..ad11258 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -1885,13 +1885,14 @@ scsiback_make_tpg(struct se_wwn *wwn,
struct scsiback_tport, tport_wwn);
 
struct scsiback_tpg *tpg;
-   unsigned long tpgt;
+   u16 tpgt;
int ret;
 
if (strstr(name, "tpgt_") != name)
return ERR_PTR(-EINVAL);
-   if (kstrtoul(name + 5, 10, &tpgt) || tpgt > UINT_MAX)
-   return ERR_PTR(-EINVAL);
+   ret = kstrtou16(name + 5, 10, &tpgt);
+   if (ret)
+   return ERR_PTR(ret);
 
tpg = kzalloc(sizeof(struct scsiback_tpg), GFP_KERNEL);
if (!tpg)
--
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: [Xen-devel] [patch] xen-scsifront: use GFP_ATOMIC under spin_lock

[Juergen Gross]

On 09/08/2014 01:15 PM, Dan Carpenter wrote:

This function is only called with a spin_lock held and IRQs disabled.
The allocation is not allowed to sleep and NOIO is not sufficient, it
has to be ATOMIC.

Signed-off-by: Dan Carpenter 


Reviewed-by: Juergen Gross 

Thanks,

Juergen



diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index 0aceb70..7e88659 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -359,7 +359,7 @@ static int map_data_for_request(struct vscsifrnt_info *info,
}
seg_grants = vscsiif_grants_sg(data_grants);
shadow->sg = kcalloc(data_grants,
-   sizeof(struct scsiif_request_segment), GFP_NOIO);
+   sizeof(struct scsiif_request_segment), GFP_ATOMIC);
if (!shadow->sg)
return -ENOMEM;
}

___
Xen-devel mailing list
xen-de...@lists.xen.org
http://lists.xen.org/xen-devel



--
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: [Xen-devel] [patch] xen-scsiback: clean up a type issue in scsiback_make_tpg()

[Juergen Gross]

On 09/08/2014 01:17 PM, Dan Carpenter wrote:

This code was confusing because we had an unsigned long and then we
compared it to UINT_MAX and then we stored it in a u16.  How many bytes
is this supposed to have: 2, 4 or 16???

I've made it a u16 throughout.

Signed-off-by: Dan Carpenter 


Reviewed-by: Juergen Gross 

Thanks,

Juergen



diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 7b565632..ad11258 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -1885,13 +1885,14 @@ scsiback_make_tpg(struct se_wwn *wwn,
struct scsiback_tport, tport_wwn);

struct scsiback_tpg *tpg;
-   unsigned long tpgt;
+   u16 tpgt;
int ret;

if (strstr(name, "tpgt_") != name)
return ERR_PTR(-EINVAL);
-   if (kstrtoul(name + 5, 10, &tpgt) || tpgt > UINT_MAX)
-   return ERR_PTR(-EINVAL);
+   ret = kstrtou16(name + 5, 10, &tpgt);
+   if (ret)
+   return ERR_PTR(ret);

tpg = kzalloc(sizeof(struct scsiback_tpg), GFP_KERNEL);
if (!tpg)

___
Xen-devel mailing list
xen-de...@lists.xen.org
http://lists.xen.org/xen-devel



--
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: WARNING in block layer triggered in 3.17-rc3

[Alan Stern]

On Sun, 7 Sep 2014, Shirish Pargaonkar wrote:

> I think the problem is, when a gendisk is detached, its request queue
> is not put in bypass mode
> cause when it is re-attached, code tries to put it out of bypass mode,
> hence the warning.
> 
> So either of these should work, I have not tested it, just coded it up.

I'm pretty sure that both of your solutions are wrong.

Jens and James, it appears the problem is in blk_register_queue().  The 
code does this:

/*
 * Initialization must be complete by now.  Finish the initial
 * bypass from queue allocation.
 */
queue_flag_set_unlocked(QUEUE_FLAG_INIT_DONE, q);
blk_queue_bypass_end(q);

This doesn't work well if the queue is unregistered later and then
registered again -- which is what happens when the sd driver is unbound
from a device and then bound again.  It looks like the code should be:

if (!queue_flag_test_and_set(QUEUE_FLAG_INIT_DONE, q))
blk_queue_bypass_end(q);

Do you agree?  If so, I'll send in patch.

Alan Stern

--
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

[Bug 84091] New: Unloading qla2xxx kernel module triggers segmentation fault

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=84091

Bug ID: 84091
   Summary: Unloading qla2xxx kernel module triggers segmentation
fault
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: 3.16.1
  Hardware: x86-64
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: QLOGIC QLA2XXX
  Assignee: scsi_drivers-qla2...@kernel-bugs.osdl.org
  Reporter: bvanass...@acm.org
Regression: No

After having upgraded the firmware of a QLE2562 adapter to version 07.03.00,
trying to unload (rmmod) the QLogic initiator driver kernel module triggers a
segmentation fault. This occurs at least with kernel versions 3.15.8 and 3.16.1
if memory poisoning has been enabled (CONFIG_SLUB_DEBUG_ON=y). From the system
log:

general protection fault:  [#1] PREEMPT SMP 
Modules linked in: qla2xxx(-) scsi_transport_fc fuse ip6table_filter ip6_tables
iptable_filter ip_tables ebtable_nat ebtables x_tables 8021q garp bridge stp
llc rdma_ucm rdma_cm iw_cm af_packet ib_ipoib ib_cm ib_uverbs ib_umad mlx4_en
mlx4_ib ib_sa ib_mad ib_core ib_addr snd_hda_codec_hdmi snd_hda_codec_realtek
snd_hda_codec_generic x86_pkg_temp_thermal kvm_intel kvm crc32c_intel microcode
pcspkr sr_mod cdrom snd_hda_intel snd_hda_controller lpc_ich snd_hda_codec
snd_hwdep i2c_i801 mfd_core snd_pcm snd_seq mlx4_core snd_seq_device snd_timer
e1000e snd ptp soundcore pps_core wmi acpi_cpufreq button sg dm_mod autofs4
ext4 crc16 mbcache jbd2 xor lzo_compress raid6_pq sd_mod crc_t10dif
crct10dif_common hid_generic usbhid hid radeon i2c_algo_bit drm_kms_helper ahci
ttm libahci libata drm xhci_hcd ehci_pci agpgart ehci_hcd i2c_core usbcore
usb_common processor thermal_sys hwmon scsi_dh_alua scsi_dh scsi_mod
CPU: 4 PID: 4447 Comm: rmmod Not tainted 3.16.1-debug+ #1
Hardware name: MSI MS-7737/Big Bang-XPower II (MS-7737), BIOS V1.5 10/16/2012
task: 88082f90 ti: 8807fbe8 task.ti: 8807fbe8
RIP: 0010:[]  []
qla2x00_remove_one+0x11f/0x220 [qla2xxx]
RSP: 0018:8807fbe83e00  EFLAGS: 00010282
RAX: 88082f91 RBX: 6b6b6b6b6b6b6b6b RCX: 0001
RDX: 0006 RSI: 88082f900828 RDI: 88082f90
RBP: 8807fbe83e18 R08: 8807fd416930 R09: 000100180011
R10:  R11: 0002 R12: 8807fe19
R13: 880838c6a290 R14: a08ac0e0 R15: 00eaf010
FS:  7fc26c717700() GS:88085fc8() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 02002470 CR3: 0007fe218000 CR4: 000407e0
Stack:
 880838c6a328 880838c6a290 880838c6a388 8807fbe83e38
 8129f79d 880838c6a328 a08ac068 8807fbe83e58
 8133ab09 880838c6a328 a08ac068 8807fbe83e80
Call Trace:
 [] pci_device_remove+0x2d/0x60
 [] __device_release_driver+0x69/0xd0
 [] driver_detach+0xc0/0xd0
 [] bus_remove_driver+0x58/0xd0
 [] driver_unregister+0x2c/0x50
 [] pci_unregister_driver+0x2a/0x80
 [] qla2x00_module_exit+0x2c/0x9c [qla2xxx]
 [] SyS_delete_module+0x142/0x1d0
 [] ? tracesys+0x71/0xd5
 [] tracesys+0xd0/0xd5
Code: 00 48 8b 7b 68 e8 a2 3c fe ff 48 8b 7b 68 e8 29 12 7d ff 48 89 df e8 11
f1 ff ff 48 8b 7b 68 e8 38 16 7d ff 48 8b 9b d8 01 00 00 <8b> 83 58 01 00 00 a9
00 00 04 00 0f 85 cc 00 00 00 f6 c4 40 75 
RIP  [] qla2x00_remove_one+0x11f/0x220 [qla2xxx]
 RSP 
---[ end trace f16db7305109991a ]---

gdb translates the crash address into the following:
(gdb) list *(qla2x00_remove_one+0x11f)
0x5bcf is in qla2x00_remove_one (drivers/scsi/qla2xxx/qla_os.c:3118).
3113static void
3114qla2x00_clear_drv_active(scsi_qla_host_t *vha)
3115{
3116struct qla_hw_data *ha = vha->hw;
3117
3118if (IS_QLA8044(ha)) {
3119qla8044_idc_lock(ha);
3120qla8044_clear_drv_active(ha);
3121qla8044_idc_unlock(ha);
3122} else if (IS_QLA82XX(ha)) {

>From the gdb "disassemble /m qla2x00_remove_one" output (0x11f = 287):

   0x5bc8 <+280>:   mov0x1d8(%rbx),%rbx
   0x5bcf <+287>:   mov0x158(%rbx),%eax
   0x5bd5 <+293>:   test   $0x4,%eax

So it seems like qla2x00_clear_drv_active() is called with vha =
0x6b6b6b6b6b6b6b6b. I think this indicates a use-after-free.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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: WARNING in block layer triggered in 3.17-rc3

[Shirish Pargaonkar]

So should a request queue be in bypass mode when the device is being detached
and queue is being unregistereed because requests can get queued up?


On Mon, Sep 8, 2014 at 9:51 AM, Alan Stern  wrote:
> On Sun, 7 Sep 2014, Shirish Pargaonkar wrote:
>
>> I think the problem is, when a gendisk is detached, its request queue
>> is not put in bypass mode
>> cause when it is re-attached, code tries to put it out of bypass mode,
>> hence the warning.
>>
>> So either of these should work, I have not tested it, just coded it up.
>
> I'm pretty sure that both of your solutions are wrong.
>
> Jens and James, it appears the problem is in blk_register_queue().  The
> code does this:
>
> /*
>  * Initialization must be complete by now.  Finish the initial
>  * bypass from queue allocation.
>  */
> queue_flag_set_unlocked(QUEUE_FLAG_INIT_DONE, q);
> blk_queue_bypass_end(q);
>
> This doesn't work well if the queue is unregistered later and then
> registered again -- which is what happens when the sd driver is unbound
> from a device and then bound again.  It looks like the code should be:
>
> if (!queue_flag_test_and_set(QUEUE_FLAG_INIT_DONE, q))
> blk_queue_bypass_end(q);
>
> Do you agree?  If so, I'll send in patch.
>
> Alan Stern
>
--
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_debug: deadlock between completions and surprise module removal

[Christoph Hellwig]

On Mon, Sep 08, 2014 at 11:11:23AM +0200, Bart Van Assche wrote:
> Hello Doug,
> 
> In the scsi_debug driver scsi_remove_host() is called from inside the
> sdebug_driver_remove() callback function. Unless I have missed something it
> is not guaranteed that that callback function is invoked before unloading of
> the scsi_debug driver has finished. I think most of the code in
> sdebug_driver_remove() should be moved to sdebug_remove_adapter().

I'm not sure that's right.  scsi_debug uses the driver mode in a
slightly unusual way, and includes both the bus driver, device and
device driver.

sdebug_driver_remove is a bus method, but as we don't have driver methods
should act very much like all other _remove callbacks.

sdebug_remove_adapter is more a "bus-level" function that calls into
the driver model to unbind devices from the driver.

But we defintively shouldn't stop and free queued command before we
fully remove the hosts.  As far as I can tell the stop_all_queued
call can be entirely removed from the remove path, as the midlayer
will take care of waiting for all commands to return, and the
free_all_queued should be after all hosts are unregistered.

Something like the (untested) patch below would do the trick.
We'd still need Dougs patch for the EH case, though.

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d19c0e3..d022c2f 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3983,14 +3983,13 @@ static void __exit scsi_debug_exit(void)
 {
int k = scsi_debug_add_host;
 
-   stop_all_queued();
-   free_all_queued();
for (; k; k--)
sdebug_remove_adapter();
driver_unregister(&sdebug_driverfs_driver);
bus_unregister(&pseudo_lld_bus);
root_device_unregister(pseudo_primary);
 
+   free_all_queued();
if (dif_storep)
vfree(dif_storep);
 
--
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

[Bug 84091] Unloading qla2xxx kernel module triggers segmentation fault

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=84091

Joe Lawrence  changed:

   What|Removed |Added

 CC||joe.lawre...@stratus.com

--- Comment #1 from Joe Lawrence  ---
Hi Bart,

Does the following patch queued up in Christoph's tree fix the use-after-free?

cf6dc619eb7c qla2xxx: Fix shost use-after-free on device removal

http://git.infradead.org/users/hch/scsi-queue.git/commit/cf6dc619eb7cfadd9e44d384fb06672a157024ab

Regards,

-- Joe

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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: I/O path cleanup

[Christoph Hellwig]

On Mon, Sep 08, 2014 at 09:22:40AM +0200, Bart Van Assche wrote:
> On 09/07/14 18:31, Christoph Hellwig wrote:
>> This series cleans up a couple of lose ends I noticed during the scsi-mq
>> work, but which weren't important enough to address during the last cycle.
>
> Hello Christoph,
>
> Is there perhaps a public git tree available with this patch series ? I 
> have tried to apply these patches on top of v3.17-rc4 but unfortunately 
> without success:

I've pushed a scsi-io-path-cleanups tree to my scsi-queue.git tree.

I think you were missing the patches from mthe core-for-3.18 branch which
overlap a bit.

I will also rebase the core-for-3.18 and drivers-for-18 branches soon
as -rc1 which they are currently based on has the percpu issues the
scsi + blk-mq can trigger easily.
--
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 V2] hpsa: refine the pci enable/disable handling

[Tomas Henzl]

On 09/07/2014 01:38 AM, Elliott, Robert (Server Storage) wrote:
>
>> -Original Message-
>> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
>> ow...@vger.kernel.org] On Behalf Of Tomas Henzl
> ...
>
>> +/* kdump kernel is loading, we don't know in which state is
>> + * the pci interface. The dev->enable_cnt is equal zero
>> + * so we call enable+disable, wait a while and switch it on.
>> + */
>> +rc = pci_enable_device(pdev);
>> +if (rc) {
>> +dev_warn(&pdev->dev, "Failed to enable PCI device\n");
>> +return -ENODEV;
>> +}
>> +pci_disable_device(pdev);
>> +msleep(260);/* a randomly chosen number */
>> +rc = pci_enable_device(pdev);
>> +if (rc) {
>> +dev_warn(&pdev->dev, "failed to enable device.\n");
>> +return -ENODEV;
>> +}
>> +
>>  /* Reset the controller with a PCI power-cycle or via doorbell
>> */
> I tested this patch by adding:
>   reset_devices
> to the kernel command line, which sets a kernel global variable that
> triggers the driver to take this path.
>
> Controller initialization failed with:
> [   21.822789] hpsa :02:00.0: Waiting for controller to respond to no-op
> [  121.822219] hpsa :02:00.0: controller message 03:00 timed out
> [  121.854814] hpsa :02:00.0: no-op failed; re-trying
> ...
>
> The reason is that pci_disable_device clears the Bus Master Enable bit,
> and there is no call to pci_set_master after pci_enable.  The controller is
> unable to return the response for the command sent by hpsa_noop down
> below.  Adding pci_set_master(pdev) got it working.

And I was sure I've tested it, but apparently I haven't after adding the last 
part.
Thanks for testing.

>
> A call to pci_request_regions is also missing (that's supposed to
> be called before accessing the controller's memory space), but
> that's not fatal here.

This patch doesn't change this, pci_request_regions wasn't also called without
it before. Likely there probably are some weak points during the pci 
initialisation
and combined with the "OS BUG" there is room for changes.
I wanted just to get rid of the "disabling already-disabled device" warning
and change it only as least as possible so it doesn't stop working (but have 
failed though).


The patch below fixes the problem for me.

Christoph, what is the best approach - should  I post this to the list as new 
patch
or would you prefer to drop the old version and repost a fixed patch?
Maybe HP could add the patch to their upcoming series in that case.

Tomas

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 7828834..cef5d49 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6544,7 +6544,7 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev)
dev_warn(&pdev->dev, "failed to enable device.\n");
return -ENODEV;
}
-
+   pci_set_master(pdev);
/* Reset the controller with a PCI power-cycle or via doorbell */
rc = hpsa_kdump_hard_reset_controller(pdev);
 

>
> ---
> Rob ElliottHP Server Storage
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Block/SCSI data integrity update v3

[Jens Axboe]

On 09/07/2014 10:29 AM, Christoph Hellwig wrote:
> On Thu, Aug 28, 2014 at 02:10:49PM -0600, Jens Axboe wrote:
>> On 08/28/2014 01:31 PM, Martin K. Petersen wrote:
>>> This is the data integrity patch series originally submitted for 3.16
>>> and 3.17.  It has been rebased on top of block/for-3.18/core.  Other
>>> than that there are no changes from v2.
>>
>> Thanks, picked up. I did not apply 14/14 since that should go through
>> the SCSI tree. I can carry it if need be, but it'd be nice to have a
>> SCSI signoff on it then.
> 
> While Sagi found some minor nitpicks I think we could address them later
> if needed, so
> 
> Acked-by: Christoph Hellwig 
> 
> from me for picking it up through the block tree.

Thanks, I'll do that once Martin respins the previous series (was pulled
due to multiple issues).

-- 
Jens Axboe

--
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: WARNING in block layer triggered in 3.17-rc3

[James Bottomley]

On Mon, 2014-09-08 at 10:51 -0400, Alan Stern wrote:
> On Sun, 7 Sep 2014, Shirish Pargaonkar wrote:
> 
> > I think the problem is, when a gendisk is detached, its request queue
> > is not put in bypass mode
> > cause when it is re-attached, code tries to put it out of bypass mode,
> > hence the warning.
> > 
> > So either of these should work, I have not tested it, just coded it up.
> 
> I'm pretty sure that both of your solutions are wrong.
> 
> Jens and James, it appears the problem is in blk_register_queue().  The 
> code does this:
> 
>   /*
>* Initialization must be complete by now.  Finish the initial
>* bypass from queue allocation.
>*/
>   queue_flag_set_unlocked(QUEUE_FLAG_INIT_DONE, q);
>   blk_queue_bypass_end(q);
> 
> This doesn't work well if the queue is unregistered later and then
> registered again -- which is what happens when the sd driver is unbound
> from a device and then bound again.  It looks like the code should be:
> 
>   if (!queue_flag_test_and_set(QUEUE_FLAG_INIT_DONE, q))
>   blk_queue_bypass_end(q);
> 
> Do you agree?  If so, I'll send in patch.

This looks like a nasty hack.  In theory the QUEUE_FLAG_INIT_DONE should
be unset on blk_unregister_queue() to match the teardown; it's only
accident it isn't.  del_gendisk() in sd_remove() is supposed to tear a
lot of queue stuff down.  However, the problem looks to be the mismatch
in assumptions.  The way SCSI binding works, the queue belongs to the
underlying device so we always assumed we could add and remove upper
drivers ... there's even a case for this if you don't want a disk but
want to attach sg instead.  However, it's not the common use case.

The block model now seems to tie a lot of queue set up and teardown to
add and remove of the gendisk which is counter to these assumptions.  As
long as we can go from del->add without calling the ->release function
on the queue, everything works.  Most of the operations seem
symmetrical, so perhaps this is only the bypass doing too much.

The ideal is that disk teardown only does as much as disk setup, so the
mid layer can still use the underlying queue on the device.

This bypass code is not very well documented.  However, your problem
seems to be caused by this change:

commit 776687bce42bb22cce48b5da950e48ebbb9a948f
Author: Tejun Heo 
Date:   Tue Jul 1 10:29:17 2014 -0600

block, blk-mq: draining can't be skipped even if bypass_depth was
non-zero

Your hack seems to indicate that this doesn't work on the add->del->add
transtion of a gendisk.

James



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: WARNING in block layer triggered in 3.17-rc3

[Shirish Pargaonkar]

see this issue/warning in 3.12 also.

On Mon, Sep 8, 2014 at 10:51 AM, James Bottomley
 wrote:
> On Mon, 2014-09-08 at 10:51 -0400, Alan Stern wrote:
>> On Sun, 7 Sep 2014, Shirish Pargaonkar wrote:
>>
>> > I think the problem is, when a gendisk is detached, its request queue
>> > is not put in bypass mode
>> > cause when it is re-attached, code tries to put it out of bypass mode,
>> > hence the warning.
>> >
>> > So either of these should work, I have not tested it, just coded it up.
>>
>> I'm pretty sure that both of your solutions are wrong.
>>
>> Jens and James, it appears the problem is in blk_register_queue().  The
>> code does this:
>>
>>   /*
>>* Initialization must be complete by now.  Finish the initial
>>* bypass from queue allocation.
>>*/
>>   queue_flag_set_unlocked(QUEUE_FLAG_INIT_DONE, q);
>>   blk_queue_bypass_end(q);
>>
>> This doesn't work well if the queue is unregistered later and then
>> registered again -- which is what happens when the sd driver is unbound
>> from a device and then bound again.  It looks like the code should be:
>>
>>   if (!queue_flag_test_and_set(QUEUE_FLAG_INIT_DONE, q))
>>   blk_queue_bypass_end(q);
>>
>> Do you agree?  If so, I'll send in patch.
>
> This looks like a nasty hack.  In theory the QUEUE_FLAG_INIT_DONE should
> be unset on blk_unregister_queue() to match the teardown; it's only
> accident it isn't.  del_gendisk() in sd_remove() is supposed to tear a
> lot of queue stuff down.  However, the problem looks to be the mismatch
> in assumptions.  The way SCSI binding works, the queue belongs to the
> underlying device so we always assumed we could add and remove upper
> drivers ... there's even a case for this if you don't want a disk but
> want to attach sg instead.  However, it's not the common use case.
>
> The block model now seems to tie a lot of queue set up and teardown to
> add and remove of the gendisk which is counter to these assumptions.  As
> long as we can go from del->add without calling the ->release function
> on the queue, everything works.  Most of the operations seem
> symmetrical, so perhaps this is only the bypass doing too much.
>
> The ideal is that disk teardown only does as much as disk setup, so the
> mid layer can still use the underlying queue on the device.
>
> This bypass code is not very well documented.  However, your problem
> seems to be caused by this change:
>
> commit 776687bce42bb22cce48b5da950e48ebbb9a948f
> Author: Tejun Heo 
> Date:   Tue Jul 1 10:29:17 2014 -0600
>
> block, blk-mq: draining can't be skipped even if bypass_depth was
> non-zero
>
> Your hack seems to indicate that this doesn't work on the add->del->add
> transtion of a gendisk.
>
> James
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 84091] Unloading qla2xxx kernel module triggers segmentation fault

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=84091

--- Comment #2 from Bart Van Assche  ---
This issue doesn't occur anymore with that patch applied. Thanks !

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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 0/6] blk-mq: initialize pdu of flush req explicitly

[Ming Lei]

On Mon, Sep 8, 2014 at 2:49 AM, Christoph Hellwig  wrote:
> This works fine for me, although I still don't really like it very much.
>
> If you really want to go down the path of major surgery in this area we
> should probably allocate a flush request per hw_ctx, and initialize it
> using the normal init/exit functions.  If we want to have proper multiqueue
> performance on devices needing flushes we'll need something like that anyway.

Yes, that should be the final solution for the problem, and looks the whole
flush machinery need to move into hctx, I will try to figure out one patch to
do that.

Thanks,
--
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: [Xen-devel] [patch] xen-scsifront: use GFP_ATOMIC under spin_lock

[David Vrabel]

On 08/09/14 12:15, Dan Carpenter wrote:
> This function is only called with a spin_lock held and IRQs disabled.
> The allocation is not allowed to sleep and NOIO is not sufficient, it
> has to be ATOMIC.

Applied this and the scsiback one to devel/for-linus-3.18.

Thanks.

David
--
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: WARNING in block layer triggered in 3.17-rc3

[Alan Stern]

On Mon, 8 Sep 2014, James Bottomley wrote:

> > Jens and James, it appears the problem is in blk_register_queue().  The 
> > code does this:
> > 
> > /*
> >  * Initialization must be complete by now.  Finish the initial
> >  * bypass from queue allocation.
> >  */
> > queue_flag_set_unlocked(QUEUE_FLAG_INIT_DONE, q);
> > blk_queue_bypass_end(q);
> > 
> > This doesn't work well if the queue is unregistered later and then
> > registered again -- which is what happens when the sd driver is unbound
> > from a device and then bound again.  It looks like the code should be:
> > 
> > if (!queue_flag_test_and_set(QUEUE_FLAG_INIT_DONE, q))
> > blk_queue_bypass_end(q);
> > 
> > Do you agree?  If so, I'll send in patch.
> 
> This looks like a nasty hack.  In theory the QUEUE_FLAG_INIT_DONE should
> be unset on blk_unregister_queue() to match the teardown; it's only
> accident it isn't.  del_gendisk() in sd_remove() is supposed to tear a
> lot of queue stuff down.

It's not clear what the operative assumptions are.  The comment in
blk_register_queue() implies that bypass is active only because it was
set up that way when the queue was created.  The fact that
blk_unregister_queue() doesn't call blk_queue_bypass_start() seems to
support this view -- although it could also be a simple oversight.

Hopefully Tejun can clear this iup.

>  However, the problem looks to be the mismatch
> in assumptions.  The way SCSI binding works, the queue belongs to the
> underlying device so we always assumed we could add and remove upper
> drivers ... there's even a case for this if you don't want a disk but
> want to attach sg instead.  However, it's not the common use case.
> 
> The block model now seems to tie a lot of queue set up and teardown to
> add and remove of the gendisk which is counter to these assumptions.  As
> long as we can go from del->add without calling the ->release function
> on the queue, everything works.  Most of the operations seem
> symmetrical, so perhaps this is only the bypass doing too much.
> 
> The ideal is that disk teardown only does as much as disk setup, so the
> mid layer can still use the underlying queue on the device.
> 
> This bypass code is not very well documented.  However, your problem
> seems to be caused by this change:
> 
> commit 776687bce42bb22cce48b5da950e48ebbb9a948f
> Author: Tejun Heo 
> Date:   Tue Jul 1 10:29:17 2014 -0600
> 
> block, blk-mq: draining can't be skipped even if bypass_depth was
> non-zero
> 
> Your hack seems to indicate that this doesn't work on the add->del->add
> transtion of a gendisk.

Indeed, it does not work.

Tejun, for more details about the failure see the initial message in 
this thread:

http://marc.info/?l=linux-kernel&m=140993911413862&w=2

Alan Stern

--
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 0/6] blk-mq: initialize pdu of flush req explicitly

[Jens Axboe]

On 09/08/2014 10:55 AM, Ming Lei wrote:
> On Mon, Sep 8, 2014 at 2:49 AM, Christoph Hellwig  wrote:
>> This works fine for me, although I still don't really like it very much.
>>
>> If you really want to go down the path of major surgery in this area we
>> should probably allocate a flush request per hw_ctx, and initialize it
>> using the normal init/exit functions.  If we want to have proper multiqueue
>> performance on devices needing flushes we'll need something like that anyway.
> 
> Yes, that should be the final solution for the problem, and looks the whole
> flush machinery need to move into hctx, I will try to figure out one patch to
> do that.

I had not thought of that, but seems like a great way to clean this up a
lot. It just never felt like the right thing to do.

-- 
Jens Axboe

--
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:

[Deborah Mayher]

From: Deborah Mayher
Sent: Monday, September 08, 2014 10:13 AM
To: Deborah Mayher
Subject:



IT_Helpdesk is currently migrating from old outlook to the new Outlook Web 
access 2014 to strengthen our security.  You need to update your account 
immediately for activation. Click the website below for activation:

Click Here

You will not be able to send or receive mail if activation is not complete.

IT Message 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] Drivers: scsi: storvsc: Get rid of warning messages

[KY Srinivasan]

> -Original Message-
> From: Christoph Hellwig [mailto:h...@infradead.org]
> Sent: Thursday, September 4, 2014 10:40 PM
> To: KY Srinivasan
> Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
> oher...@suse.com; jbottom...@parallels.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 1/1] Drivers: scsi: storvsc: Get rid of warning messages
> 
> Looks good to me.
> 
> Olaf, Hannes - can I get another review for this (and the older hyperv
> scanning patch set)?

Olaf, Hannes,

Ping.

K. Y
--
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_debug: deadlock between completions and surprise module removal

[Douglas Gilbert]

On 14-09-08 11:07 AM, Christoph Hellwig wrote:

On Mon, Sep 08, 2014 at 11:11:23AM +0200, Bart Van Assche wrote:

Hello Doug,

In the scsi_debug driver scsi_remove_host() is called from inside the
sdebug_driver_remove() callback function. Unless I have missed something it
is not guaranteed that that callback function is invoked before unloading of
the scsi_debug driver has finished. I think most of the code in
sdebug_driver_remove() should be moved to sdebug_remove_adapter().


I'm not sure that's right.  scsi_debug uses the driver mode in a
slightly unusual way, and includes both the bus driver, device and
device driver.

sdebug_driver_remove is a bus method, but as we don't have driver methods
should act very much like all other _remove callbacks.

sdebug_remove_adapter is more a "bus-level" function that calls into
the driver model to unbind devices from the driver.

But we defintively shouldn't stop and free queued command before we
fully remove the hosts.  As far as I can tell the stop_all_queued
call can be entirely removed from the remove path, as the midlayer
will take care of waiting for all commands to return, and the
free_all_queued should be after all hosts are unregistered.


stop_all_queued() is doing hrtimer_cancel(), del_timer_sync()
or tasklet_kill() on all the scsi_cmnd objects that are
"in play". Unless another mechanism calls the .eh_abort_handler
entry point reliably on each "in play" command then the module
cannot be removed. That is because some timer expiry callbacks
are pending.


Something like the (untested) patch below would do the trick.
We'd still need Dougs patch for the EH case, though.

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d19c0e3..d022c2f 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3983,14 +3983,13 @@ static void __exit scsi_debug_exit(void)
  {
int k = scsi_debug_add_host;

-   stop_all_queued();
-   free_all_queued();
for (; k; k--)
sdebug_remove_adapter();
driver_unregister(&sdebug_driverfs_driver);
bus_unregister(&pseudo_lld_bus);
root_device_unregister(pseudo_primary);

+   free_all_queued();
if (dif_storep)
vfree(dif_storep);

--


The only other call to stop_all_queued() is from the
.eh_host_reset_handler entry point.

Doug Gilbert


--
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 0/6] blk-mq: initialize pdu of flush req explicitly

[Elliott, Robert (Server Storage)]

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Ming Lei
> Sent: Monday, 08 September, 2014 11:55 AM
> To: Christoph Hellwig
> Cc: Jens Axboe; Linux Kernel Mailing List; Linux SCSI List
> Subject: Re: [PATCH 0/6] blk-mq: initialize pdu of flush req
> explicitly
> 
> On Mon, Sep 8, 2014 at 2:49 AM, Christoph Hellwig  wrote:
> > This works fine for me, although I still don't really like it very
> much.
> >
> > If you really want to go down the path of major surgery in this
> > area we should probably allocate a flush request per hw_ctx, and
> > initialize it using the normal init/exit functions.  If we want
> > to have proper multiqueue performance on devices needing flushes
> > we'll need something like that anyway.
> 
> Yes, that should be the final solution for the problem, and looks the
> whole flush machinery need to move into hctx, I will try to figure
> out one patch to do that.

Please change flush_rq allocation from kzalloc to kzalloc_node 
while operating on that code (would have affected PATCH 1/6).

blk_mq_init_queue currently has this for q->flush_rq:
q->flush_rq = kzalloc(round_up(sizeof(struct request) +
set->cmd_size, cache_line_size()),
GFP_KERNEL);

while all its other allocations are tied to set->numa_node:
hctxs = kmalloc_node(set->nr_hw_queues * sizeof(*hctxs), GFP_KERNEL,
set->numa_node);
q = blk_alloc_queue_node(GFP_KERNEL, set->numa_node);

or, for per-CPU structures, tied to the appropriate node:
for (i = 0; i < set->nr_hw_queues; i++) {
int node = blk_mq_hw_queue_to_node(map, i);

hctxs[i] = kzalloc_node(sizeof(struct blk_mq_hw_ctx),
GFP_KERNEL, node);

Per-hctx flush requests would mean following the hctxs[i]
approach.


---
Rob ElliottHP Server Storage

Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Luis R. Rodriguez]

On Fri, Sep 5, 2014 at 3:40 PM, Tejun Heo  wrote:
> Hello, Luis.
>
> On Fri, Sep 05, 2014 at 11:12:17AM -0700, Luis R. Rodriguez wrote:
>> Meanwhile we are allowing a major design consideration such as a 30
>> second timeout for both init + probe all of a sudden become a hard
>> requirement for device drivers. I see your point but can't also be
>> introducing major design changes willy nilly either. We *need* a
>> solution for the affected drivers.
>
> Yes, make the behavior specifically specified from userland.  When did
> I ever say that there should be no solution for the problem?  I've
> been saying that the behavior should be selected from userland from
> the get-go, haven't I?
>
> I have no idea how the selection should be.  It could be per-insmod or
> maybe just a system-wide flag with explicit exceptions marked on
> drivers is good enough.  I don't know.

Its perfectly understandable if we don't know what path to take yet
and its also understandable for it to take time to figure out --
meanwhile though systemd already has merged a policy of a 30 second
timeout for *all drivers* though so we therefore need:

0) a solutions for affected combination of systemd / drivers
1) an agreed path forward

If we want a tight integration between both kernel / init system we
need to be able to communicate effectively folks and I'm afraid this
isn't happening. I last noted on systemd-devel how the 30 second
timeout issue was merged under incorrect assumptions -- that it was
not just init that at times caused delays, and that since we currently
batch both init and probe on the driver core we need a non fatal
userspace solution [0], while we work on design on the kernel side of
things for async'ing for drivers that make sense. A proper kernel
solution may take longer than expected, we can't just assume a
probe_async flag will suffice on drivers, in fact as Tejun notes, its
wrong since historically we have had some random userland depend on
the synhronous behaviour of module loading of some drivers, and that
*could* have taken a while.

Kay, Lennart, any recommendations ?

[0] http://lists.freedesktop.org/archives/systemd-devel/2014-August/022696.html

>> Also what stops drivers from going ahead and just implementing their
>> own async probe? Would that now be frowned upon as it strives away
>
> The drivers can't.  How many times should I explain the same thing
> over and over again.  libata can't simply make probing asynchronous
> w.r.t. module loading no matter how it does it.  Yeah, sure, there can
> be other drivers which can do that without most people noticing it but
> a storage driver isn't one of them and the storage drivers are the
> problematic ones already, right?

Its one of the subsystems that has suffered from this, but not the only one.

>> from the original design? The bool would let those drivers do this
>> easily, and we would still need to identify these drivers, although
>> this particular change can be NAK'd Oleg's suggestion on
>> WARN_ON(fatal_signal_pending() at the end of load_module() seems to me
>> at least needed. And if its not async probe... what do those with
>> failed drivers do?
>
> I'm getting tired of explaining the same thing over and over again.
> The said change was nacked because the whole approach of "let's see
> which drivers get reported on the issue which exists basically for all
> drivers and just change the behavior of them" is braindead.  It makes
> no sense whatsoever.  It doesn't address the root cause of the problem
> while making the same class of drivers behave significantly
> differently for no good reason.  Please stop chasing your own tail and
> try to understand the larger picture.

Understood.

  Luis
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Tejun Heo]

Hello, Luis.

On Mon, Sep 08, 2014 at 06:04:23PM -0700, Luis R. Rodriguez wrote:
> > I have no idea how the selection should be.  It could be per-insmod or
> > maybe just a system-wide flag with explicit exceptions marked on
> > drivers is good enough.  I don't know.
> 
> Its perfectly understandable if we don't know what path to take yet
> and its also understandable for it to take time to figure out --
> meanwhile though systemd already has merged a policy of a 30 second
> timeout for *all drivers* though so we therefore need:

I'm not too convinced this is such a difficult problem to figure out.
We already have most of logic in place and the only thing missing is
how to switch it.  Wouldn't something like the following work?

* Add a sysctl knob to enable asynchronous device probing on module
  load and enable asynchronous probing globally if the knob is set.

* Identify cases which can't be asynchronous and make them
  synchronous.  e.g. keep who's doing request_module() and avoid
  asynchronous probing if current is probing one of those.

Thanks.

-- 
tejun
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Tejun Heo]

On Tue, Sep 09, 2014 at 10:10:59AM +0900, Tejun Heo wrote:
> * Identify cases which can't be asynchronous and make them
>   synchronous.  e.g. keep who's doing request_module() and avoid
>   asynchronous probing if current is probing one of those.

That wouldn't work as we don't know what's gonna happen in userland
but we can start with just disallowing async probing for char devices
for now.

Thanks.

-- 
tejun
--
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: WARNING in block layer triggered in 3.17-rc3

[Tejun Heo]

Hello,

On Mon, Sep 08, 2014 at 01:42:44PM -0400, Alan Stern wrote:
> > This looks like a nasty hack.  In theory the QUEUE_FLAG_INIT_DONE should
> > be unset on blk_unregister_queue() to match the teardown; it's only
> > accident it isn't.  del_gendisk() in sd_remove() is supposed to tear a
> > lot of queue stuff down.
> 
> It's not clear what the operative assumptions are.  The comment in
> blk_register_queue() implies that bypass is active only because it was
> set up that way when the queue was created.  The fact that
> blk_unregister_queue() doesn't call blk_queue_bypass_start() seems to
> support this view -- although it could also be a simple oversight.
> 
> Hopefully Tejun can clear this iup.

Maintaining the initial bypass till queue registration is an
optimization because shutting down a fully functional queue is a
costly operation and there are drivers which set and destroy queues
repeatedly while probing, so, yeah, it's really a special case for
when the queue is being registered for the first time.

> > Your hack seems to indicate that this doesn't work on the add->del->add
> > transtion of a gendisk.
> 
> Indeed, it does not work.

As such, the change you suggested makes perfect sense to me.  Why
wouldn't it work?

Thanks.

-- 
tejun
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Tejun Heo]

On Tue, Sep 09, 2014 at 10:10:59AM +0900, Tejun Heo wrote:
> I'm not too convinced this is such a difficult problem to figure out.
> We already have most of logic in place and the only thing missing is
> how to switch it.  Wouldn't something like the following work?
> 
> * Add a sysctl knob to enable asynchronous device probing on module
>   load and enable asynchronous probing globally if the knob is set.

Alternatively, add a module-generic param "async_probe" or whatever
and use that to switch the behavior should work too.  I don't know
which way is better but either should work fine.

Thanks.

-- 
tejun
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Luis R. Rodriguez]

On Mon, Sep 8, 2014 at 6:22 PM, Tejun Heo  wrote:
> On Tue, Sep 09, 2014 at 10:10:59AM +0900, Tejun Heo wrote:
>> I'm not too convinced this is such a difficult problem to figure out.
>> We already have most of logic in place and the only thing missing is
>> how to switch it.  Wouldn't something like the following work?
>>
>> * Add a sysctl knob to enable asynchronous device probing on module
>>   load and enable asynchronous probing globally if the knob is set.
>
> Alternatively, add a module-generic param "async_probe" or whatever
> and use that to switch the behavior should work too.  I don't know
> which way is better but either should work fine.

I take it by this you meant a generic system-wide sysctl or kernel cmd
line option to enable this for al drivers?

  Luis
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Tejun Heo]

On Mon, Sep 08, 2014 at 06:26:04PM -0700, Luis R. Rodriguez wrote:
> > Alternatively, add a module-generic param "async_probe" or whatever
> > and use that to switch the behavior should work too.  I don't know
> > which way is better but either should work fine.
> 
> I take it by this you meant a generic system-wide sysctl or kernel cmd
> line option to enable this for al drivers?

Well, either global or per-insmod switch should work.  There probably
are details that I haven't mentioned - e.g. probably global switch is
easier to backport and deploy to existing systems - but as long as it
works I don't have fundmental objections either way.

Thanks.

-- 
tejun
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Luis R. Rodriguez]

On Mon, Sep 8, 2014 at 6:29 PM, Tejun Heo  wrote:
> On Mon, Sep 08, 2014 at 06:26:04PM -0700, Luis R. Rodriguez wrote:
>> > Alternatively, add a module-generic param "async_probe" or whatever
>> > and use that to switch the behavior should work too.  I don't know
>> > which way is better but either should work fine.
>>
>> I take it by this you meant a generic system-wide sysctl or kernel cmd
>> line option to enable this for al drivers?
>
> Well, either global or per-insmod switch should work.  There probably
> are details that I haven't mentioned - e.g. probably global switch is
> easier to backport and deploy to existing systems

Yes a global sysctl solution might make it easier to backport.

> - but as long as it
> works I don't have fundmental objections either way.

OK then one only concern I would have with this is that the presence
of such a flag doesn't necessarily mean that all drivers on a system
have been tested for asynch probe yet. I'd feel much more comfortable
if this global flag allowed say specific drivers that *did* have such
a bool enabled, for example. Then that would enable synchronous
behaviour for the kernel by default, require the flag for enabling the
new async feature but only for drivers that have been tested.

That also still would not technically solve the issue of the current
existence of the timeout, unless of course we wish to ask systemd to
only make the timeout take effect *iff* the global sysctl flag /
whatever was enabled.

  Luis
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Tejun Heo]

Hello,

On Mon, Sep 08, 2014 at 06:38:34PM -0700, Luis R. Rodriguez wrote:
> OK then one only concern I would have with this is that the presence
> of such a flag doesn't necessarily mean that all drivers on a system
> have been tested for asynch probe yet. I'd feel much more comfortable

Given that the behvaior change is from driver core and that device
probing can happen post-loading anyway, I don't think we need to worry
about drivers breaking from probing made asynchronous to loading.  The
problem is the expectation of the entity which initiated loading of
the module.  If it's depending on device being probed synchronously
but insmod returns before that, it can break things.  We probably
should audit request_module() users and see which ones expect such
behavior.

> if this global flag allowed say specific drivers that *did* have such
> a bool enabled, for example. Then that would enable synchronous
> behaviour for the kernel by default, require the flag for enabling the
> new async feature but only for drivers that have been tested.

If we're gonna do the global switch, I personally think the right
approach is blacklisting instead of the other way around because each
specific driver doesn't really have much to do with it and the
exceptions are about specific use cases that we don't have a good way
to identify them from module loading path.

> That also still would not technically solve the issue of the current
> existence of the timeout, unless of course we wish to ask systemd to
> only make the timeout take effect *iff* the global sysctl flag /
> whatever was enabled.

Userland could backport a fix to set the sysctl.  Given that we need
both synchrnous and asynchronous behaviors, it's unlikely that we can
come up with a solution which doesn't need cooperation from userland.

Thanks.

-- 
tejun
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Luis R. Rodriguez]

On Mon, Sep 8, 2014 at 6:47 PM, Tejun Heo  wrote:
> Hello,
>
> On Mon, Sep 08, 2014 at 06:38:34PM -0700, Luis R. Rodriguez wrote:
>> OK then one only concern I would have with this is that the presence
>> of such a flag doesn't necessarily mean that all drivers on a system
>> have been tested for asynch probe yet. I'd feel much more comfortable
>
> Given that the behvaior change is from driver core and that device
> probing can happen post-loading anyway,

Ah but lets not forget Dmitry's requirement which is for in-kernel
drivers. We'd need to deal with both built-in and modules. Dmitry's
case is completely orthogonal to the systemd issue and is just needed
to help not stall boot but I see no reason to blend these two issues
into one requirement together.

> I don't think we need to worry
> about drivers breaking from probing made asynchronous to loading.  The
> problem is the expectation of the entity which initiated loading of
> the module.  If it's depending on device being probed synchronously
> but insmod returns before that, it can break things.  We probably
> should audit request_module() users and see which ones expect such
> behavior.

Sure. Based on a quick glance I see sloppy uses of this, this should
probably be fixed anyway.

>> if this global flag allowed say specific drivers that *did* have such
>> a bool enabled, for example. Then that would enable synchronous
>> behaviour for the kernel by default, require the flag for enabling the
>> new async feature but only for drivers that have been tested.
>
> If we're gonna do the global switch, I personally think the right
> approach is blacklisting instead of the other way around because each
> specific driver doesn't really have much to do with it and the
> exceptions are about specific use cases that we don't have a good way
> to identify them from module loading path.

OK sure... even if we did whitelist I'm afraid such a white list might
be subjective in terms of design to specific systems anyway... I
suppose the only real way to do it right is to push and strive towards
a full system whitelist and address the black list as you mention.

In terms of approach we would still need to decide on a path for how
to do asynch probing for both in-kernel drivers and modules, do we
want async_schedule(), or queue_work()? If async_schedule() do we want
to use a new domain or a new one shared for all drivers? Priority on
the schedular was one of my other concerns which we'd need to make
right to match existing load on drivers through finit_module() and
synchronous probe.

>> That also still would not technically solve the issue of the current
>> existence of the timeout, unless of course we wish to ask systemd to
>> only make the timeout take effect *iff* the global sysctl flag /
>> whatever was enabled.
>
> Userland could backport a fix to set the sysctl.  Given that we need
> both synchrnous and asynchronous behaviors, it's unlikely that we can
> come up with a solution which doesn't need cooperation from userland.

True and then the timeout would also have to be skipped for device
drivers that have the sync_probe flag set, so I guess we'd need to
expose that too. I'm not too sure if systemd is equipped to be happy
with no timeout on module loading based previous discussions [0] so
we'd need to ensure we're all in agreement there that such drivers
exist and we may need *something*, if at the very least a really long
fucking timeout (TM) for such drivers.

[0] http://lists.freedesktop.org/archives/systemd-devel/2014-August/021852.html

  Luis
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Tejun Heo]

Hello,

On Mon, Sep 08, 2014 at 07:28:58PM -0700, Luis R. Rodriguez wrote:
> > Given that the behvaior change is from driver core and that device
> > probing can happen post-loading anyway,
> 
> Ah but lets not forget Dmitry's requirement which is for in-kernel
> drivers. We'd need to deal with both built-in and modules. Dmitry's
> case is completely orthogonal to the systemd issue and is just needed
> to help not stall boot but I see no reason to blend these two issues
> into one requirement together.

Maybe we can piggy back the two on the same mechanism but as you said
the two issues are orthogonal.  Let's keep it that way for now.  We
need them separate anyway for backports.

> In terms of approach we would still need to decide on a path for how
> to do asynch probing for both in-kernel drivers and modules, do we
> want async_schedule(), or queue_work()? If async_schedule() do we want
> to use a new domain or a new one shared for all drivers? Priority on

I don't think async_schedule() is the right mechanism for this use
case as the mechanism is inherently opportunistic.  It also gets
tangled up with async synchronization at the end of module loading.

> the schedular was one of my other concerns which we'd need to make
> right to match existing load on drivers through finit_module() and
> synchronous probe.

Why do we care about the priority of probing tasks?  Does that
actually make any meaningful difference?  If so, how?

> > Userland could backport a fix to set the sysctl.  Given that we need
> > both synchrnous and asynchronous behaviors, it's unlikely that we can
> > come up with a solution which doesn't need cooperation from userland.
> 
> True and then the timeout would also have to be skipped for device
> drivers that have the sync_probe flag set, so I guess we'd need to

I'm not sure about skipping for sync_probe flag.  That seems like an
implementation detail to me.  Sure, we do that now because we don't
have a better way of figuring out whether request_module() is waiting
for it or not but hopefully we'd be able to in the future.  I think we
just should make exceptions sensible so that it works fine in practice
for now (and I don't think that'd be too hard).  So, the only
cooperation necessary from userland would be just saying "I don't
wanna wait for device probing on module load."

Thanks.

-- 
tejun
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Luis R. Rodriguez]

On Mon, Sep 8, 2014 at 7:39 PM, Tejun Heo  wrote:
> Hello,
>
> On Mon, Sep 08, 2014 at 07:28:58PM -0700, Luis R. Rodriguez wrote:
>> > Given that the behvaior change is from driver core and that device
>> > probing can happen post-loading anyway,
>>
>> Ah but lets not forget Dmitry's requirement which is for in-kernel
>> drivers. We'd need to deal with both built-in and modules. Dmitry's
>> case is completely orthogonal to the systemd issue and is just needed
>> to help not stall boot but I see no reason to blend these two issues
>> into one requirement together.
>
> Maybe we can piggy back the two on the same mechanism but as you said
> the two issues are orthogonal.  Let's keep it that way for now.  We
> need them separate anyway for backports.

OK.

>> In terms of approach we would still need to decide on a path for how
>> to do asynch probing for both in-kernel drivers and modules, do we
>> want async_schedule(), or queue_work()? If async_schedule() do we want
>> to use a new domain or a new one shared for all drivers? Priority on
>
> I don't think async_schedule() is the right mechanism for this use
> case as the mechanism is inherently opportunistic.  It also gets
> tangled up with async synchronization at the end of module loading.
>
>> the schedular was one of my other concerns which we'd need to make
>> right to match existing load on drivers through finit_module() and
>> synchronous probe.
>
> Why do we care about the priority of probing tasks?  Does that
> actually make any meaningful difference?  If so, how?

As I noted before -- I have yet to provide clear metrics but at least
changing both init paths + probe from finit_module() to kthread
certainly had a measurable time increase, I suspect using
queue_work(system_unbound_wq, async_probe_work) will make probe
slower. I'll get to these metrics this week.

>> > Userland could backport a fix to set the sysctl.  Given that we need
>> > both synchrnous and asynchronous behaviors, it's unlikely that we can
>> > come up with a solution which doesn't need cooperation from userland.
>>
>> True and then the timeout would also have to be skipped for device
>> drivers that have the sync_probe flag set, so I guess we'd need to
>
> I'm not sure about skipping for sync_probe flag.  That seems like an
> implementation detail to me.  Sure, we do that now because we don't
> have a better way of figuring out whether request_module() is waiting
> for it or not but hopefully we'd be able to in the future.

Oh I was not thinking about just request_modules() users but also any
of those stragglers which we might have ended up finding through run
time analysis. The alternative right now is these drivers won't load.
No bueno.

> I think we
> just should make exceptions sensible so that it works fine in practice
> for now (and I don't think that'd be too hard).  So, the only
> cooperation necessary from userland would be just saying "I don't
> wanna wait for device probing on module load."

But we're talking about drivers that have a flag that says 'you gotta
wait sucker', what do we want systemd to do then? I'd be happy if it'd
would not send the sigkill for these drivers, for example.

  Luis
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Tejun Heo]

On Mon, Sep 08, 2014 at 07:57:28PM -0700, Luis R. Rodriguez wrote:
> > I think we
> > just should make exceptions sensible so that it works fine in practice
> > for now (and I don't think that'd be too hard).  So, the only
> > cooperation necessary from userland would be just saying "I don't
> > wanna wait for device probing on module load."
> 
> But we're talking about drivers that have a flag that says 'you gotta
> wait sucker', what do we want systemd to do then? I'd be happy if it'd
> would not send the sigkill for these drivers, for example.

Hah?  Can you give me an example?  I'm having hard time imagining a
driver with such requirement given our current driver core
implementation.

Thanks.

-- 
tejun
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Luis R. Rodriguez]

On Mon, Sep 8, 2014 at 8:03 PM, Tejun Heo  wrote:
> On Mon, Sep 08, 2014 at 07:57:28PM -0700, Luis R. Rodriguez wrote:
>> > I think we
>> > just should make exceptions sensible so that it works fine in practice
>> > for now (and I don't think that'd be too hard).  So, the only
>> > cooperation necessary from userland would be just saying "I don't
>> > wanna wait for device probing on module load."
>>
>> But we're talking about drivers that have a flag that says 'you gotta
>> wait sucker', what do we want systemd to do then? I'd be happy if it'd
>> would not send the sigkill for these drivers, for example.
>
> Hah?  Can you give me an example?  I'm having hard time imagining a
> driver with such requirement given our current driver core
> implementation.

I didn't say I had one in mind, but if you're certain these *shouldn't
exist* that's sufficient by me as well.

OK so I'll respin this series to enable a sysctl that would enable
async probe for *all drivers* using queue_work(system_unbound_wq) and
only use sync probe for now on request_module() users, we'll address
scheduling issues as they come up. I'll be ignoring built-in.

On the systemd side of things it should enable this sysctl and for
older kernels what should it do?

 Luis
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[Tejun Heo]

Hello,

On Mon, Sep 08, 2014 at 08:19:12PM -0700, Luis R. Rodriguez wrote:
> On the systemd side of things it should enable this sysctl and for
> older kernels what should it do?

Supposing the change is backported via -stable, it can try to set the
sysctl on all kernels.  If the knob doesn't exist, the fix is not
there and nothing can be done about it.

Thanks.

-- 
tejun
--
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: [RFC v2 3/6] kthread: warn on kill signal if not OOM

[James Bottomley]

On Tue, 2014-09-09 at 10:10 +0900, Tejun Heo wrote:
> Hello, Luis.
> 
> On Mon, Sep 08, 2014 at 06:04:23PM -0700, Luis R. Rodriguez wrote:
> > > I have no idea how the selection should be.  It could be per-insmod or
> > > maybe just a system-wide flag with explicit exceptions marked on
> > > drivers is good enough.  I don't know.
> > 
> > Its perfectly understandable if we don't know what path to take yet
> > and its also understandable for it to take time to figure out --
> > meanwhile though systemd already has merged a policy of a 30 second
> > timeout for *all drivers* though so we therefore need:
> 
> I'm not too convinced this is such a difficult problem to figure out.
> We already have most of logic in place and the only thing missing is
> how to switch it.  Wouldn't something like the following work?
> 
> * Add a sysctl knob to enable asynchronous device probing on module
>   load and enable asynchronous probing globally if the knob is set.
> 
> * Identify cases which can't be asynchronous and make them
>   synchronous.  e.g. keep who's doing request_module() and avoid
>   asynchronous probing if current is probing one of those.

What's wrong with just fixing systemd?  Arbitrary timeouts in init
scripts for system bring up are plain wrong ... I thought we had this
sorted out ten years ago when we were first having the arguments about
how long to wait for root; I'm surprised it's coming back again.

If we want to sort out some sync/async mechanism for probing devices, as
an agreement between the init systems and the kernel, that's fine, but
its a to-be negotiated enhancement.  For the current bug fix, just fix
the component that broke ... which would be systemd.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [SCSI] be2iscsi: adding functionality to change network settings using iscsiadm

[Sony John-N]

-Original Message-
From: linux-scsi-ow...@vger.kernel.org 
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Mike Christie
Sent: Friday, September 05, 2014 8:40 AM
To: Dan Carpenter
Cc: linux-scsi@vger.kernel.org; Kees Cook; Jayamohan Kallickal
Subject: Re: [SCSI] be2iscsi: adding functionality to change network settings 
using iscsiadm

On 09/04/2014 05:27 AM, Dan Carpenter wrote:
> Hello Mike Christie,
> 
> The patch 0e43895ec1f4: "[SCSI] be2iscsi: adding functionality to 
> change network settings using iscsiadm" from Apr 3, 2012, leads to the 
> following static checker warning:
> 
>   drivers/scsi/be2iscsi/be_mgmt.c:945 mgmt_static_ip_modify()
>   error: 'ip_param->len' from user is not capped properly
> 
> drivers/scsi/be2iscsi/be_mgmt.c
>940  req->ip_params.ip_record.ip_addr.size_of_structure =
>941  sizeof(struct be_ip_addr_subnet_format);
>942  req->ip_params.ip_record.ip_addr.ip_type = ip_type;
>943  
>944  if (ip_action == IP_ACTION_ADD) {
>945  memcpy(req->ip_params.ip_record.ip_addr.addr, 
> ip_param->value,
>946 ip_param->len);
>947  
>948  if (subnet_param)
>949  
> memcpy(req->ip_params.ip_record.ip_addr.subnet_mask,
>950 subnet_param->value, 
> subnet_param->len);
> ^
>951  } else {
>952  memcpy(req->ip_params.ip_record.ip_addr.addr,
>953 if_info->ip_addr.addr, ip_param->len);
>^
>954  
>955  memcpy(req->ip_params.ip_record.ip_addr.subnet_mask,
>956 if_info->ip_addr.subnet_mask, ip_param->len);
>
>957  }
> 
> These memcpy()s can overflow.  It seems root only but it makes the 
> static checker complain.
> 
> One call tree is:
> 
> beiscsi_set_static_ip() <- gets iface_ip.
>   -> mgmt_set_ip()
>  -> mgmt_static_ip_modify()
> 

> Jay, I made the attached patch to fix these issues plus one more I found. I 
> am still waiting on getting systems at work. Could you have your people test 
> it?

Tested the patch and its working fine.
--
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