diff mbox

Kernel oops

Message ID CAAKD3BDFrTMMgX0nErD50rp2je=HC9zeaYWHDKf0mqQwc5fM9g@mail.gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Matan Barak July 27, 2017, 12:54 p.m. UTC
On Thu, Jul 27, 2017 at 2:46 PM, Matan Barak <matanb@dev.mellanox.co.il> wrote:
> On Tue, Jul 25, 2017 at 12:16 AM, Jason Gunthorpe
> <jgunthorpe@obsidianresearch.com> wrote:
>> Matan,
>>
>> I suspect your reworking series broke hot removal, I get a kernel oops
>> when removing modules on 4.13-rc2.
>>
>> I think it is some kind of race with uverbs being removed concurrently
>> with user apps closing uverbs. In this case I removed umad first which
>> caused systemd to forcibly shutdown srp_daemon, which happened
>> concurrently with the rmmod of ib_uverbs.
>>
>
> Hi,
>
> Thanks for informing :)
> We'll try to debug that, but I expect we'll only get to that by end of
> next week.
>
> Thanks,
> Matan
>

Digging a bit, we found a fix that might be related to this issue.
I would be happy if you could try that and report if it solved this problem.
We plan to send it soon.

commit 1d4ecbf034193f000fe6686586c40ab4b2a95da1
Author: Yishai Hadas <yishaih@mellanox.com>
Date:   Thu Jul 27 15:49:00 2017 +0200

    IB/uverbs: Fix device cleanup

    Uverbs device should be cleaned up only when there is no
    potential usage of.

    As part of ib_uverbs_remove_one which might be triggered upon reset flow
    the device reference count is decreased as expected and leave the final
    cleanup to the FDs that were opened.

    Current code increases reference count upon opening a new command FD and
    decreases it upon closing the file. The event FD is opened internally
    and rely on the command FD by taking on it a reference count.

    In case that the command FD was closed and just later the event FD we
    may ensure that the device resources as of srcu are still alive as they
    are still in use.

    Fixing the above by moving the reference count decreasing to the place
    where the command FD is really freed instead of doing that when it was
    just closed.

    Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
    Reviewed-by: Matan Barak <matanb@mellanox.com>

        if (file->ucontext) {
@@ -940,7 +940,6 @@ static int ib_uverbs_close(struct inode *inode,
struct file *filp)
                         ib_uverbs_release_async_event_file);

        kref_put(&file->ref, ib_uverbs_release_file);
-       kobject_put(&dev->kobj);

        return 0;
 }


>> [   50.797421] general protection fault: 0000 [#1] SMP
>> [   50.798400] Modules linked in: ib_srp scsi_transport_srp scsi_mod rdma_cm ib_umad ib_cm ib_uverbs iw_cm mlx4_core ib_core [last unloaded: mlx4_ib]
>> [   50.800946] CPU: 0 PID: 235 Comm: srp_daemon Not tainted 4.13.0-rc2+ #2
>> [   50.802178] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
>> [   50.803970] task: ffff88022e504b00 task.stack: ffffc90000178000
>> [   50.805163] RIP: 0010:ib_uverbs_release_file+0x29/0x90 [ib_uverbs]
>> [   50.806428] RSP: 0018:ffffc9000017bd90 EFLAGS: 00010202
>> [   50.807521] RAX: 0000000000000001 RBX: ffff88022dec56c0 RCX: 0000000000000001
>> [   50.808898] RDX: 732f74696e752f31 RSI: ffff88022dec56c0 RDI: ffff88022cccc000
>> [   50.810349] RBP: ffffc9000017bda0 R08: 000000002dec5801 R09: 0000000180150013
>> [   50.811830] R10: ffffc9000017bd80 R11: ffff88022afe5200 R12: ffff88022dec56c0
>> [   50.813260] R13: ffff88022dec56e8 R14: ffff88022dec5f70 R15: ffff88022edaa020
>> [   50.814830] FS:  00007fb54d287700(0000) GS:ffff880236e00000(0000) knlGS:0000000000000000
>> [   50.816401] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [   50.817602] CR2: 00007fb54bc4c8f8 CR3: 000000022dcd9000 CR4: 00000000000406b0
>> [   50.818991] Call Trace:
>> [   50.819487]  uverbs_close_fd+0x5f/0xa0 [ib_uverbs]
>> [   50.820416]  ib_uverbs_comp_event_close+0xa4/0xb0 [ib_uverbs]
>> [   50.821539]  __fput+0xd4/0x1d0
>> [   50.822202]  ____fput+0x9/0x10
>> [   50.822890]  task_work_run+0x79/0xa0
>> [   50.823676]  do_exit+0x362/0xa90
>> [   50.824309]  ? __do_page_fault+0x203/0x430
>> [   50.825140]  do_group_exit+0x42/0xb0
>> [   50.825908]  SyS_exit_group+0xf/0x10
>> [   50.826630]  entry_SYSCALL_64_fastpath+0x1a/0xa5
>> [   50.827591] RIP: 0033:0x7fb54c73bb98
>> [   50.828340] RSP: 002b:00007ffec1987788 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
>> [   50.829900] RAX: ffffffffffffffda RBX: 0000006aa1fe2dc0 RCX: 00007fb54c73bb98
>> [   50.831303] RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
>> [   50.832793] RBP: 00007ffec1987780 R08: 00000000000000e7 R09: ffffffffffffff98
>> [   50.834288] R10: 00007fb54a446640 R11: 0000000000000246 R12: 00007fb54be4dc50
>> [   50.835802] R13: 00000000ffffffff R14: 00007ffec1987680 R15: 0000000000000006
>> [   50.837254] Code: 1f 00 55 48 89 e5 53 48 89 fb 48 83 ec 08 48 8b 47 48 48 8d b8 10 01 00 00 e8 d4 f8 02 e1 48 8b 7b 48 48 8b 57 30 48 85 d2 74 0a <48> 83 ba b0 02 00 00 00 74 42 89 c6 48 81 c7 10 01 00 00 e8 df
>> [   50.841156] RIP: ib_uverbs_release_file+0x29/0x90 [ib_uverbs] RSP: ffffc9000017bd90
>> [   50.842723] ---[ end trace 68785d98b53d9203 ]---
>>
>> Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jason Gunthorpe July 27, 2017, 8:44 p.m. UTC | #1
On Thu, Jul 27, 2017 at 03:54:07PM +0300, Matan Barak wrote:

> Digging a bit, we found a fix that might be related to this issue.
> I would be happy if you could try that and report if it solved this problem.
> We plan to send it soon.

Yep this looks like it.

FWIW, it causes random kernel memory corruption and failures in my
experience, I was very lucky to get such a clean oops the first time..

> commit 1d4ecbf034193f000fe6686586c40ab4b2a95da1
> Author: Yishai Hadas <yishaih@mellanox.com>
> Date:   Thu Jul 27 15:49:00 2017 +0200
> 
>     IB/uverbs: Fix device cleanup
> 
>     Uverbs device should be cleaned up only when there is no
>     potential usage of.
> 
>     As part of ib_uverbs_remove_one which might be triggered upon reset flow
>     the device reference count is decreased as expected and leave the final
>     cleanup to the FDs that were opened.
> 
>     Current code increases reference count upon opening a new command FD and
>     decreases it upon closing the file. The event FD is opened internally
>     and rely on the command FD by taking on it a reference count.
> 
>     In case that the command FD was closed and just later the event FD we
>     may ensure that the device resources as of srcu are still alive as they
>     are still in use.
> 
>     Fixing the above by moving the reference count decreasing to the place
>     where the command FD is really freed instead of doing that when it was
>     just closed.
> 
>     Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
>     Reviewed-by: Matan Barak <matanb@mellanox.com>

Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Please add a fixes line

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky July 30, 2017, 10:25 a.m. UTC | #2
On Thu, Jul 27, 2017 at 02:44:37PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 27, 2017 at 03:54:07PM +0300, Matan Barak wrote:
>
> > Digging a bit, we found a fix that might be related to this issue.
> > I would be happy if you could try that and report if it solved this problem.
> > We plan to send it soon.
>
> Yep this looks like it.
>
> FWIW, it causes random kernel memory corruption and failures in my
> experience, I was very lucky to get such a clean oops the first time..
>
> > commit 1d4ecbf034193f000fe6686586c40ab4b2a95da1
> > Author: Yishai Hadas <yishaih@mellanox.com>
> > Date:   Thu Jul 27 15:49:00 2017 +0200
> >
> >     IB/uverbs: Fix device cleanup
> >
> >     Uverbs device should be cleaned up only when there is no
> >     potential usage of.
> >
> >     As part of ib_uverbs_remove_one which might be triggered upon reset flow
> >     the device reference count is decreased as expected and leave the final
> >     cleanup to the FDs that were opened.
> >
> >     Current code increases reference count upon opening a new command FD and
> >     decreases it upon closing the file. The event FD is opened internally
> >     and rely on the command FD by taking on it a reference count.
> >
> >     In case that the command FD was closed and just later the event FD we
> >     may ensure that the device resources as of srcu are still alive as they
> >     are still in use.
> >
> >     Fixing the above by moving the reference count decreasing to the place
> >     where the command FD is really freed instead of doing that when it was
> >     just closed.
> >
> >     Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> >     Reviewed-by: Matan Barak <matanb@mellanox.com>
>
> Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>
> Please add a fixes line

Hi Jason,

I queued it [1] for submission, once the IPoIB fixes [2] will be
accepted, I'll submit it.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git/commit/?h=rdma-rc&id=38a974d578451dbbde0c40fc2d81fba44027a338
[2] http://marc.info/?l=linux-rdma&m=150109276402195&w=2

>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe July 31, 2017, 3:52 a.m. UTC | #3
On Sun, Jul 30, 2017 at 01:25:14PM +0300, Leon Romanovsky wrote:
> > Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >
> > Please add a fixes line
> 
> Hi Jason,
> 
> I queued it [1] for submission, once the IPoIB fixes [2] will be
> accepted, I'll submit it.

Isn't fixing random kernel memory corruption triggerable by userspace
exactly the sort of thing we should be rushing into Linus's tree?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky July 31, 2017, 5:39 a.m. UTC | #4
On Sun, Jul 30, 2017 at 09:52:08PM -0600, Jason Gunthorpe wrote:
> On Sun, Jul 30, 2017 at 01:25:14PM +0300, Leon Romanovsky wrote:
> > > Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > > Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > >
> > > Please add a fixes line
> >
> > Hi Jason,
> >
> > I queued it [1] for submission, once the IPoIB fixes [2] will be
> > accepted, I'll submit it.
>
> Isn't fixing random kernel memory corruption triggerable by userspace
> exactly the sort of thing we should be rushing into Linus's tree?

I'm still looking on the easiest way to submit patches and want to see
that everything is working before "rushing".

Thanks

>
> Jason
Leon Romanovsky July 31, 2017, 7:12 a.m. UTC | #5
On Mon, Jul 31, 2017 at 08:39:01AM +0300, Leon Romanovsky wrote:
> On Sun, Jul 30, 2017 at 09:52:08PM -0600, Jason Gunthorpe wrote:
> > On Sun, Jul 30, 2017 at 01:25:14PM +0300, Leon Romanovsky wrote:
> > > > Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > > > Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > > >
> > > > Please add a fixes line
> > >
> > > Hi Jason,
> > >
> > > I queued it [1] for submission, once the IPoIB fixes [2] will be
> > > accepted, I'll submit it.
> >
> > Isn't fixing random kernel memory corruption triggerable by userspace
> > exactly the sort of thing we should be rushing into Linus's tree?
>
> I'm still looking on the easiest way to submit patches and want to see
> that everything is working before "rushing".

OK, I followed the advice [1] - "I would prefer to see is one submission,
then two or three days (so the first submission has had some bake time),
then the next one and it should assume the first is applied."

And submitted the fix [2].

[1] http://marc.info/?l=linux-rdma&m=150020922014003&w=2
[2] https://patchwork.kernel.org/patch/9871109/

>
> Thanks
>
> >
> > Jason
diff mbox

Patch

diff --git a/drivers/infiniband/core/uverbs_main.c
b/drivers/infiniband/core/uverbs_main.c
index a88d223..cb1729a 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -251,6 +251,7 @@  void ib_uverbs_release_file(struct kref *ref)
        if (atomic_dec_and_test(&file->device->refcount))
                ib_uverbs_comp_dev(file->device);

+       kobject_put(&file->device->kobj);
        kfree(file);
 }

@@ -918,7 +919,6 @@  static int ib_uverbs_open(struct inode *inode,
struct file *filp)
 static int ib_uverbs_close(struct inode *inode, struct file *filp)
 {
        struct ib_uverbs_file *file = filp->private_data;
-       struct ib_uverbs_device *dev = file->device;

        mutex_lock(&file->cleanup_mutex);