Re: [PATCH] IB/srpt: Revert "Convert to percpu_ida tag allocation"

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2846 bytes --]

On 4/7/16 4:52 PM, Nicholas A. Bellinger wrote:
> On Thu, 2016-04-07 at 16:44 -0700, Bart Van Assche wrote:
>> On 04/07/2016 04:37 PM, Nicholas A. Bellinger wrote:
>>> On Thu, 2016-04-07 at 15:55 -0700, Bart Van Assche wrote:
>>>> That patch causes the ib_srpt driver to crash as soon as the first
>>>> SCSI command is received. This means that that patch was untested.
>>>> Hence revert it. The shortcomings of that patch are as follows:
>>>> - It makes the ib_srpt driver use I/O contexts allocated by
>>>>    transport_alloc_session_tags() but it does not initialize these
>>>>    I/O contexts properly. All the initializations performed by
>>>>    srpt_alloc_ioctx() are skipped.
>>>> - It swaps the order of the send ioctx allocation and the transition
>>>>    to RTR mode which is wrong.
>>>> - The amount of memory that is needed for I/O contexts is doubled.
>>>> - srpt_rdma_ch.free_list is no longer used but is not removed.
>>>>
>>>> Revert commit 0fd10721fe36 and thereby fix the following kernel crash:
>>>>
>>>> kernel BUG at drivers/infiniband/ulp/srpt/ib_srpt.c:1439!
>>>> invalid opcode: 0000 [#1] SMP
>>>> Workqueue: target_completion target_complete_ok_work [target_core_mod]
>>>> RIP: 0010:[<ffffffffa052ef37>]  [<ffffffffa052ef37>] srpt_queue_response+0x437/0x4a0 [ib_srpt]
>>>> Call Trace:
>>>>   [<ffffffffa052f009>] srpt_queue_data_in+0x9/0x10 [ib_srpt]
>>>>   [<ffffffffa04f1ee2>] target_complete_ok_work+0x152/0x2b0 [target_core_mod]
>>>>   [<ffffffff81071ea7>] process_one_work+0x197/0x480
>>>>   [<ffffffff810721d9>] worker_thread+0x49/0x490
>>>>   [<ffffffff8107878a>] kthread+0xea/0x100
>>>>   [<ffffffff8159b172>] ret_from_fork+0x22/0x40
>>>>
>>>> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>>>> Cc: Nicholas Bellinger <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>
>>>
>>> I've already asked you not to revert the patch [ ... ]
>>
>> But why not to revert that patch? Your patch was completely untested and 
>> makes the ib_srpt driver unusable. I think the regular approach for such 
>> patches is to revert them and to rework these in a later kernel version. 
>> Linus, please let me know if you disagree.
> 
> Because I'm actively working on a bug-fix for the regression, and it's
> -rc2 in a target driver that hardly anyone cares about.

I care about this driver.  It's what we use internally for all of our
SRP testing as we don't have and of the big DDN arrays that provide an
SRP targets, so we built our own so we could test our clients.

> If the regression is not addressed before v4.6-rc6, then by all means
> revert the original patch.
> 
> 
> 


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>    GPG Key ID: 0E572FDD
  Red Hat, Inc.
  100 E. Davie St
  Raleigh, NC 27601 USA


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 907 bytes --]