Re: [PATCH 1/2] sysfs: Invoke iomem_get_mapping() from the sysfs open callback

[Dan Williams <dan.j.williams@intel.com>]

On Mon, Jun 28, 2021 at 11:07 AM Krzysztof Wilczyński <kw@linux.com> wrote:
>
> Hi Dan,
>
> On 21-06-28 10:36:13, Dan Williams wrote:
> > On Mon, Jun 28, 2021 at 3:12 AM Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > > On Fri, Jun 25, 2021 at 11:31:17PM +0000, Krzysztof Wilczy??ski wrote:
> > > >       if (battr->mapping)
> > > > -             of->file->f_mapping = battr->mapping;
> > > > +             of->file->f_mapping = battr->mapping();
> > >
> > > I think get_mapping() is a better name now.  That being said this
> > > whole programming model looks a little weird.
> >
> > I think both those points are fair.
>
> Anything for us to do?
>
> > > Also, does this patch imply the mapping field of the sysfs bin
> > > attributes wasn't used before at all?
> >
> > It defaulted to an address_space per file rather than a shared address
> > space across all files that map physical addresses as file offsets.
>
> I will include this in the commit message for v3 of the patch series, if
> you don't mind.  Also, would this shared address space be a potential
> issue?  Security, functional, etc.

The shared address_space arrangement is what allows for a "revoke"
mechanism for /dev/mem and pci-resource-sysfs mappings. Without a
shared address space there would need to be tracking for each 'inode'
instance to run revoke_iomem(). Note that this shared address_space
scheme is also deployed for block-device special files, see
blkdev_open(). It's done for similar reasons of allowing all
address_space operations to act on a common representation of the
single block-device.