Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Thu, Dec 13, 2012 at 01:08:54AM +0100, Hans J. Koch wrote: > On Wed, Dec 12, 2012 at 07:08:18AM -0800, Greg KH wrote: > > On Wed, Dec 12, 2012 at 09:56:16AM +0100, Benedikt Spranger wrote: > > > Am Wed, 12 Dec 2012 09:50:54 +0100 > > > schrieb "Hans J. Koch" : > > > > > > > On Tue, Dec 11, 2012 at 08:46:48PM -0800, Greg KH wrote: > > > > > Yes, but what does that have to do with this in-kernel, internal api? > > > > > > > > Ah, OK. You're right, the commit message is confusing. > > > > > > > > Bene, it's enough to say we drop the inode parameter because nobody > > > > ever needed it. > > > I am fine with that. > > > > > > > I cannot see why this also helps with the other problem. > > > It would help, because we can defer calling the release hook until the > > > last mmap user is gone. In this case the inode pointer may not be valid > > > anymore. > > > > Which, again, is the same for any in-kernel driver with these types of > > callbacks. > > Is that a general mmap problem that wants to be fixed? Not that I know of, but I haven't messed around in this area of the kernel in a long time, sorry. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Wed, Dec 12, 2012 at 07:08:18AM -0800, Greg KH wrote: > On Wed, Dec 12, 2012 at 09:56:16AM +0100, Benedikt Spranger wrote: > > Am Wed, 12 Dec 2012 09:50:54 +0100 > > schrieb "Hans J. Koch" : > > > > > On Tue, Dec 11, 2012 at 08:46:48PM -0800, Greg KH wrote: > > > > Yes, but what does that have to do with this in-kernel, internal api? > > > > > > Ah, OK. You're right, the commit message is confusing. > > > > > > Bene, it's enough to say we drop the inode parameter because nobody > > > ever needed it. > > I am fine with that. > > > > > I cannot see why this also helps with the other problem. > > It would help, because we can defer calling the release hook until the > > last mmap user is gone. In this case the inode pointer may not be valid > > anymore. > > Which, again, is the same for any in-kernel driver with these types of > callbacks. Is that a general mmap problem that wants to be fixed? hjk > > greg k-h > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Wed, Dec 12, 2012 at 09:56:16AM +0100, Benedikt Spranger wrote: > Am Wed, 12 Dec 2012 09:50:54 +0100 > schrieb "Hans J. Koch" : > > > On Tue, Dec 11, 2012 at 08:46:48PM -0800, Greg KH wrote: > > > Yes, but what does that have to do with this in-kernel, internal api? > > > > Ah, OK. You're right, the commit message is confusing. > > > > Bene, it's enough to say we drop the inode parameter because nobody > > ever needed it. > I am fine with that. > > > I cannot see why this also helps with the other problem. > It would help, because we can defer calling the release hook until the > last mmap user is gone. In this case the inode pointer may not be valid > anymore. Which, again, is the same for any in-kernel driver with these types of callbacks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
Am Wed, 12 Dec 2012 09:50:54 +0100 schrieb "Hans J. Koch" : > On Tue, Dec 11, 2012 at 08:46:48PM -0800, Greg KH wrote: > > Yes, but what does that have to do with this in-kernel, internal api? > > Ah, OK. You're right, the commit message is confusing. > > Bene, it's enough to say we drop the inode parameter because nobody > ever needed it. I am fine with that. > I cannot see why this also helps with the other problem. It would help, because we can defer calling the release hook until the last mmap user is gone. In this case the inode pointer may not be valid anymore. Regards Bene -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Tue, Dec 11, 2012 at 08:46:48PM -0800, Greg KH wrote:
> On Wed, Dec 12, 2012 at 02:42:22AM +0100, Hans J. Koch wrote:
> > On Tue, Dec 11, 2012 at 03:20:32PM -0800, Greg KH wrote:
> > > On Wed, Dec 12, 2012 at 12:12:02AM +0100, Benedikt Spranger wrote:
> > > > The inode parameter is unused by in kernel users of UIO.
> > >
> > > Ok.
> > >
> > > > Also the inode parameter makes it hard to resolve the existing open(),
> > > > mmap() and close() difficulty.
> > >
> > > I don't understand, what do you mean by this? What is this parameter
> > > causing problems with?
> >
> > The problem is that according to POSIX, it is guaranteed that in userspace
> > you can do
> >
> > fd = open("/dev/uio0", ...)
> > ptr = mmap(...fd...)
> > close(fd)
> >
> > with ptr still being valid and useable after that.
>
> Yes, but what does that have to do with this in-kernel, internal api?
Ah, OK. You're right, the commit message is confusing.
Bene, it's enough to say we drop the inode parameter because nobody
ever needed it. I cannot see why this also helps with the other problem.
Thanks,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Tue, Dec 11, 2012 at 08:46:48PM -0800, Greg KH wrote: On Wed, Dec 12, 2012 at 02:42:22AM +0100, Hans J. Koch wrote: On Tue, Dec 11, 2012 at 03:20:32PM -0800, Greg KH wrote: On Wed, Dec 12, 2012 at 12:12:02AM +0100, Benedikt Spranger wrote: The inode parameter is unused by in kernel users of UIO. Ok. Also the inode parameter makes it hard to resolve the existing open(), mmap() and close() difficulty. I don't understand, what do you mean by this? What is this parameter causing problems with? The problem is that according to POSIX, it is guaranteed that in userspace you can do fd = open(/dev/uio0, ...) ptr = mmap(...fd...) close(fd) with ptr still being valid and useable after that. Yes, but what does that have to do with this in-kernel, internal api? Ah, OK. You're right, the commit message is confusing. Bene, it's enough to say we drop the inode parameter because nobody ever needed it. I cannot see why this also helps with the other problem. Thanks, Hans -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
Am Wed, 12 Dec 2012 09:50:54 +0100 schrieb Hans J. Koch h...@hansjkoch.de: On Tue, Dec 11, 2012 at 08:46:48PM -0800, Greg KH wrote: Yes, but what does that have to do with this in-kernel, internal api? Ah, OK. You're right, the commit message is confusing. Bene, it's enough to say we drop the inode parameter because nobody ever needed it. I am fine with that. I cannot see why this also helps with the other problem. It would help, because we can defer calling the release hook until the last mmap user is gone. In this case the inode pointer may not be valid anymore. Regards Bene -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Wed, Dec 12, 2012 at 09:56:16AM +0100, Benedikt Spranger wrote: Am Wed, 12 Dec 2012 09:50:54 +0100 schrieb Hans J. Koch h...@hansjkoch.de: On Tue, Dec 11, 2012 at 08:46:48PM -0800, Greg KH wrote: Yes, but what does that have to do with this in-kernel, internal api? Ah, OK. You're right, the commit message is confusing. Bene, it's enough to say we drop the inode parameter because nobody ever needed it. I am fine with that. I cannot see why this also helps with the other problem. It would help, because we can defer calling the release hook until the last mmap user is gone. In this case the inode pointer may not be valid anymore. Which, again, is the same for any in-kernel driver with these types of callbacks. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Wed, Dec 12, 2012 at 07:08:18AM -0800, Greg KH wrote: On Wed, Dec 12, 2012 at 09:56:16AM +0100, Benedikt Spranger wrote: Am Wed, 12 Dec 2012 09:50:54 +0100 schrieb Hans J. Koch h...@hansjkoch.de: On Tue, Dec 11, 2012 at 08:46:48PM -0800, Greg KH wrote: Yes, but what does that have to do with this in-kernel, internal api? Ah, OK. You're right, the commit message is confusing. Bene, it's enough to say we drop the inode parameter because nobody ever needed it. I am fine with that. I cannot see why this also helps with the other problem. It would help, because we can defer calling the release hook until the last mmap user is gone. In this case the inode pointer may not be valid anymore. Which, again, is the same for any in-kernel driver with these types of callbacks. Is that a general mmap problem that wants to be fixed? hjk greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Thu, Dec 13, 2012 at 01:08:54AM +0100, Hans J. Koch wrote: On Wed, Dec 12, 2012 at 07:08:18AM -0800, Greg KH wrote: On Wed, Dec 12, 2012 at 09:56:16AM +0100, Benedikt Spranger wrote: Am Wed, 12 Dec 2012 09:50:54 +0100 schrieb Hans J. Koch h...@hansjkoch.de: On Tue, Dec 11, 2012 at 08:46:48PM -0800, Greg KH wrote: Yes, but what does that have to do with this in-kernel, internal api? Ah, OK. You're right, the commit message is confusing. Bene, it's enough to say we drop the inode parameter because nobody ever needed it. I am fine with that. I cannot see why this also helps with the other problem. It would help, because we can defer calling the release hook until the last mmap user is gone. In this case the inode pointer may not be valid anymore. Which, again, is the same for any in-kernel driver with these types of callbacks. Is that a general mmap problem that wants to be fixed? Not that I know of, but I haven't messed around in this area of the kernel in a long time, sorry. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Wed, Dec 12, 2012 at 02:42:22AM +0100, Hans J. Koch wrote:
> On Tue, Dec 11, 2012 at 03:20:32PM -0800, Greg KH wrote:
> > On Wed, Dec 12, 2012 at 12:12:02AM +0100, Benedikt Spranger wrote:
> > > The inode parameter is unused by in kernel users of UIO.
> >
> > Ok.
> >
> > > Also the inode parameter makes it hard to resolve the existing open(),
> > > mmap() and close() difficulty.
> >
> > I don't understand, what do you mean by this? What is this parameter
> > causing problems with?
>
> The problem is that according to POSIX, it is guaranteed that in userspace
> you can do
>
> fd = open("/dev/uio0", ...)
> ptr = mmap(...fd...)
> close(fd)
>
> with ptr still being valid and useable after that.
Yes, but what does that have to do with this in-kernel, internal api?
confused,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Tue, Dec 11, 2012 at 03:20:32PM -0800, Greg KH wrote:
> On Wed, Dec 12, 2012 at 12:12:02AM +0100, Benedikt Spranger wrote:
> > The inode parameter is unused by in kernel users of UIO.
>
> Ok.
>
> > Also the inode parameter makes it hard to resolve the existing open(),
> > mmap() and close() difficulty.
>
> I don't understand, what do you mean by this? What is this parameter
> causing problems with?
The problem is that according to POSIX, it is guaranteed that in userspace
you can do
fd = open("/dev/uio0", ...)
ptr = mmap(...fd...)
close(fd)
with ptr still being valid and useable after that.
Thanks,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Wed, Dec 12, 2012 at 12:12:02AM +0100, Benedikt Spranger wrote: > The inode parameter is unused by in kernel users of UIO. Ok. > Also the inode parameter makes it hard to resolve the existing open(), > mmap() and close() difficulty. I don't understand, what do you mean by this? What is this parameter causing problems with? greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] uio: do not expose inode to uio open/release hooks
The inode parameter is unused by in kernel users of UIO. Also the inode
parameter makes it hard to resolve the existing open(), mmap() and close()
difficulty.
Signed-off-by: Benedikt Spranger
---
Documentation/DocBook/uio-howto.tmpl |4 ++--
drivers/uio/uio.c|4 ++--
include/linux/uio_driver.h |4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/DocBook/uio-howto.tmpl
b/Documentation/DocBook/uio-howto.tmpl
index 59a886d..589992e 100644
--- a/Documentation/DocBook/uio-howto.tmpl
+++ b/Documentation/DocBook/uio-howto.tmpl
@@ -490,14 +490,14 @@ instead of the built-in one.
-int (*open)(struct uio_info *info, struct inode *inode)
+int (*open)(struct uio_info *info)
: Optional. You might want to have your own
open(), e.g. to enable interrupts only when your
device is actually used.
-int (*release)(struct uio_info *info, struct inode *inode)
+int (*release)(struct uio_info *info)
: Optional. If you define your own
release(), you will probably also want a custom
release() function.
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 5110f36..bbffc38 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -465,7 +465,7 @@ static int uio_open(struct inode *inode, struct file *filep)
filep->private_data = listener;
if (idev->info->open) {
- ret = idev->info->open(idev->info, inode);
+ ret = idev->info->open(idev->info);
if (ret)
goto err_infoopen;
}
@@ -496,7 +496,7 @@ static int uio_release(struct inode *inode, struct file
*filep)
struct uio_device *idev = listener->dev;
if (idev->info->release)
- ret = idev->info->release(idev->info, inode);
+ ret = idev->info->release(idev->info);
module_put(idev->owner);
kfree(listener);
diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
index 1ad4724..ad59ded 100644
--- a/include/linux/uio_driver.h
+++ b/include/linux/uio_driver.h
@@ -92,8 +92,8 @@ struct uio_info {
void*priv;
irqreturn_t (*handler)(int irq, struct uio_info *dev_info);
int (*mmap)(struct uio_info *info, struct vm_area_struct *vma);
- int (*open)(struct uio_info *info, struct inode *inode);
- int (*release)(struct uio_info *info, struct inode *inode);
+ int (*open)(struct uio_info *info);
+ int (*release)(struct uio_info *info);
int (*irqcontrol)(struct uio_info *info, s32 irq_on);
};
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] uio: do not expose inode to uio open/release hooks
The inode parameter is unused by in kernel users of UIO. Also the inode
parameter makes it hard to resolve the existing open(), mmap() and close()
difficulty.
Signed-off-by: Benedikt Spranger b.spran...@linutronix.de
---
Documentation/DocBook/uio-howto.tmpl |4 ++--
drivers/uio/uio.c|4 ++--
include/linux/uio_driver.h |4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/DocBook/uio-howto.tmpl
b/Documentation/DocBook/uio-howto.tmpl
index 59a886d..589992e 100644
--- a/Documentation/DocBook/uio-howto.tmpl
+++ b/Documentation/DocBook/uio-howto.tmpl
@@ -490,14 +490,14 @@ instead of the built-in one.
/para/listitem
listitempara
-varnameint (*open)(struct uio_info *info, struct inode *inode)
+varnameint (*open)(struct uio_info *info)
/varname: Optional. You might want to have your own
functionopen()/function, e.g. to enable interrupts only when your
device is actually used.
/para/listitem
listitempara
-varnameint (*release)(struct uio_info *info, struct inode *inode)
+varnameint (*release)(struct uio_info *info)
/varname: Optional. If you define your own
functionrelease()/function, you will probably also want a custom
functionrelease()/function function.
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 5110f36..bbffc38 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -465,7 +465,7 @@ static int uio_open(struct inode *inode, struct file *filep)
filep-private_data = listener;
if (idev-info-open) {
- ret = idev-info-open(idev-info, inode);
+ ret = idev-info-open(idev-info);
if (ret)
goto err_infoopen;
}
@@ -496,7 +496,7 @@ static int uio_release(struct inode *inode, struct file
*filep)
struct uio_device *idev = listener-dev;
if (idev-info-release)
- ret = idev-info-release(idev-info, inode);
+ ret = idev-info-release(idev-info);
module_put(idev-owner);
kfree(listener);
diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
index 1ad4724..ad59ded 100644
--- a/include/linux/uio_driver.h
+++ b/include/linux/uio_driver.h
@@ -92,8 +92,8 @@ struct uio_info {
void*priv;
irqreturn_t (*handler)(int irq, struct uio_info *dev_info);
int (*mmap)(struct uio_info *info, struct vm_area_struct *vma);
- int (*open)(struct uio_info *info, struct inode *inode);
- int (*release)(struct uio_info *info, struct inode *inode);
+ int (*open)(struct uio_info *info);
+ int (*release)(struct uio_info *info);
int (*irqcontrol)(struct uio_info *info, s32 irq_on);
};
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Wed, Dec 12, 2012 at 12:12:02AM +0100, Benedikt Spranger wrote: The inode parameter is unused by in kernel users of UIO. Ok. Also the inode parameter makes it hard to resolve the existing open(), mmap() and close() difficulty. I don't understand, what do you mean by this? What is this parameter causing problems with? greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Tue, Dec 11, 2012 at 03:20:32PM -0800, Greg KH wrote: On Wed, Dec 12, 2012 at 12:12:02AM +0100, Benedikt Spranger wrote: The inode parameter is unused by in kernel users of UIO. Ok. Also the inode parameter makes it hard to resolve the existing open(), mmap() and close() difficulty. I don't understand, what do you mean by this? What is this parameter causing problems with? The problem is that according to POSIX, it is guaranteed that in userspace you can do fd = open(/dev/uio0, ...) ptr = mmap(...fd...) close(fd) with ptr still being valid and useable after that. Thanks, Hans -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] uio: do not expose inode to uio open/release hooks
On Wed, Dec 12, 2012 at 02:42:22AM +0100, Hans J. Koch wrote: On Tue, Dec 11, 2012 at 03:20:32PM -0800, Greg KH wrote: On Wed, Dec 12, 2012 at 12:12:02AM +0100, Benedikt Spranger wrote: The inode parameter is unused by in kernel users of UIO. Ok. Also the inode parameter makes it hard to resolve the existing open(), mmap() and close() difficulty. I don't understand, what do you mean by this? What is this parameter causing problems with? The problem is that according to POSIX, it is guaranteed that in userspace you can do fd = open(/dev/uio0, ...) ptr = mmap(...fd...) close(fd) with ptr still being valid and useable after that. Yes, but what does that have to do with this in-kernel, internal api? confused, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
