lkml.org 
[lkml]   [2020]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 14/17] resource: Move devmem revoke code to resource framework
    On Fri, Oct 9, 2020 at 2:31 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
    >
    > On Fri, Oct 09, 2020 at 09:59:31AM +0200, Daniel Vetter wrote:
    >
    > > +struct address_space *iomem_get_mapping(void)
    > > +{
    > > + return iomem_inode->i_mapping;
    >
    > This should pair an acquire with the release below
    >
    > > + /*
    > > + * Publish /dev/mem initialized.
    > > + * Pairs with smp_load_acquire() in revoke_iomem().
    > > + */
    > > + smp_store_release(&iomem_inode, inode);
    >
    > However, this seems abnormal, initcalls rarely do this kind of stuff
    > with global data..
    >
    > The kernel crashes if this fs_initcall is raced with
    > iomem_get_mapping() due to the unconditional dereference, so I think
    > it can be safely switched to a simple assignment.

    Ah yes I checked this all, but forgot to correctly annotate the
    iomem_get_mapping access. For reference, see b34e7e298d7a ("/dev/mem:
    Add missing memory barriers for devmem_inode").

    The reasons for the annotations is that iomem requests can happen
    fairly early, way before fs_initcalls happen. That means revoke_iomem
    needs to check for that and bail out if we race - nothing bad can
    happen since userspace isn't running at this point anyway. And
    apparently it needs to be a full acquire fence since we don't just
    write a value, but need a barrier for the struct stuff.

    Now iomem_get_mapping otoh can only be called after userspace is up &
    running, so way after all the fs_initcalls are guaranteed to have
    fininshed. Hence we don't really need anything there. But I expect the
    kernel race checker thing to complain, plus that then gives me a good
    spot to explain why we can't race and don't have to check for a NULL
    iomem_inode.

    I'll add that in v3.
    -Daniel
    --
    Daniel Vetter
    Software Engineer, Intel Corporation
    http://blog.ffwll.ch

    \
     
     \ /
      Last update: 2020-10-09 16:25    [W:4.033 / U:0.132 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site