On Mon, Jun 28, 2021 at 11:07 AM Krzysztof Wilczyński <kw@xxxxxxxxx> 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@xxxxxxxxxxxxx> 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.