Date:   Wed, 27 Sep 2017 08:41:12 +0200
From:   Federico Vaga <federico.vaga@...a.pv.it>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Tejun Heo <tj@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: kernFS/sysfs: mmap and vm_operations close

On Tuesday, 26 September 2017 23:31:29 CEST Greg Kroah-Hartman wrote:
> On Tue, Sep 26, 2017 at 05:50:55PM +0200, Federico Vaga wrote:
> > Hello,
> > 
> > I'm writing a sysfs binary attribute that makes use of the `mmap`
> > operation.
> Eeek, why?  What are you using that for?

I have a bus (VME) and we are not using the the VME subsystem provided by the 
kernel (why? Historical reasons, we had our own implementation before the 
kernel one appeared and we cannot port hundreds of drivers so easily in our 
environment).
I want to make our VME interface "as PCI as possible", so I want to re-create 
resources that users can `mmap` to access the device memory.

This abstraction will allow us to write utilities that works on PCI and VME 
devices without dealing with the peculiarity of each interface. This is 
particularly useful when you have FPGAs that can run the "same" code on 
different buses, but it is true as well when you have the same hardware (the 
same memory map) installed on VME cards or PCI cards.

That's why a sysfs VME resource seems to me the easiest and clean way to 
achieve this. I do not want to create yet another layer that hides the 
differences between the two buses when mmap is so straight forward and easy to 
implement. Another point is that adding a new layer add complexity in the 
architecture and for the developers; they have to learn yet another non-
standard interface that I invent, while the concept of resource is something 
that everybody know.

> sysfs binary attributes are for dumping binary data that the kernel
> doesn't touch/parse, through to hardware.  Why use mmap for this?  Do
> you have a pointer to your code somewhere?

No pointer :S
But the code is not far from the Linux VME subsystem implementation since all 
the VME bus implementations comes from the same original code.
They differs in interfaces with the users and the drivers.

> > I would like to implement my own `open()` and `close()` `vm_ops` but
> > apparently I'm not allowed to do it.
> 
> Nope, you are not, not in sysfs :)
> 
> > -------- kernfs/file.c - kernfs_fop_mmap () - modern kernel -----
> > -------- sysfs/bin.c - mmap () - old kernel -----
> > 
> >         /*
> >         
> >          * It is not possible to successfully wrap close.
> >          * So error if someone is trying to use close.
> >          */
> >         
> >         rc = -EINVAL;
> >         if (vma->vm_ops && vma->vm_ops->close)
> >         
> >                 goto out_put;
> > 
> > ----------------------------------------------------------
> > 
> > What is the reason behind this choice?
> > Why "it is not possible to successfully wrap close" ?
> 
> Because you shouldn't be doing that :)

I know that this answer can take a lot of time ... but this is not an answer 
:D

Anyway, don't waste time in answering, it is a personal curiosity because I do 
not see immediately why "it is not possible".

> 
> > Is there an alternative/hack in order to be notified when the mmap is not
> > used anymore and I can properly release my resources?
> 
> Don't use mmap in sysfs :)
> 
> > Due to HW resources limitation I "cannot" keep the device memory mapped
> > when nobody is using it, that's why I would like to be able to use
> > vm_ops->close(). In general, I would like to run my routine that release
> > resources when the user does `munmap` or `close`
> 
> Don't use mmap in sysfs :)

ehehe I see. Is the sysfs binary attribute mmap a sort of a trap? :D

Why do we have it if we should not use it? Of course we cannot remove it 
because we have very important users of it (PCI). But then why it is not 
stated that the use of  mmap is not suggested?

> What problem are you trying to solve here that mmap seemed like the
> correct solution?

Answered above

Thank you

-- 
Federico Vaga
http://www.federicovaga.it

Powered by blists - more mailing lists