Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
Bill Irwin <[EMAIL PROTECTED]> writes: > On Wed, Mar 07, 2007 at 04:03:17PM -0700, Eric W. Biederman wrote: >> I think the right answer is most likely to add an extra file method or >> two so we can remove the need for is_file_hugepages. >> There are still 4 calls to is_file_hugepages in ipc/shm.c and >> 2 calls in mm/mmap.c not counting the one in is_file_shm_hugepages. >> The special cases make it difficult to properly wrap hugetlbfs files >> with another file, which is why we have the weird special case above. > > It's not clear to me that the core can be insulated from hugetlb's > distinct pagecache and memory mapping granularities in a Linux-native > manner, but if you come up with something new or manage to get the > known methods past Linus, akpm, et al, more power to you. I will agree with that there are limits on what can be achieved. However looking at where we have tests for is_file_hugepages most of those tests don't appear to be inherently anything to do with huge pages, so it wouldn't surprise me if we couldn't generalize things a little more. > I'm not entirely sure what you're up to, but I'm mostly here to sanction > others' design notions since my own are far too extreme, and, of course, > review and ack patches, take bugreports and write fixes (not that I've > managed to get to any of them first in a long while, if ever), and so on. > I say killing the is_whatever_hugepages() checks with whatever abstraction > is good, since I don't like them myself, provided it's sane. Go for it. Mostly I had reference counting and consistency problems with ipc/shm.c that had horrible leak potential when I exited a ipc namespace. Implementing everything as stacked files made the code simpler and more maintainable. (shm_nattach stopped being a special case yea!) I'm happy to stop here but if someone cares to proceed with removing is_file_hugepages I want to encourage that. I don't see any other cleanups short of that are really worth doing. Everything in ipc/shm.c could be considered a weird special case, so I'm not going to worry about it too much. Although removing those special cases is good. There is some odd accounting logic in mm/mmap.c based on is_file_hugepages and there is the get_unmapped_area case. For get_unmapped_area I see no reason to presume that the only kind of file that must live at a specific address are huge pages (even if that is the only kind of file where we have that case today). So generalizing that check should be relatively straight forward. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
On Wed, Mar 07, 2007 at 05:26:48PM -0600, Adam Litke wrote: > :) Enter my remove-is_file_hugepages() patches (which I posted a few > weeks ago). I'll rework them and repost soon. That should help to make > all of this cleaner. Those were great. I've wanted something like them for a long, long time. -- wli - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
Bill Irwin <[EMAIL PROTECTED]> writes: >> A comment to prepare others for the impending doubletake might be nice. >> Or maybe just open-coding the equality check for _file_operations >> in is_file_shm_hugepages() if others find it as jarring as I. Please >> extend my ack to any follow-up fiddling with that. On Wed, Mar 07, 2007 at 04:03:17PM -0700, Eric W. Biederman wrote: > You did notice we are testing a different struct file? Yes. Bill Irwin <[EMAIL PROTECTED]> writes: >> The patch addresses relatively straightforward issues and naturally at >> that. On Wed, Mar 07, 2007 at 04:03:17PM -0700, Eric W. Biederman wrote: > The whole concept is recursive so I'm not certain being a recursive check > is that bad but I understand the point. Hence my characterization as natural. You did notice this was an ack? On Wed, Mar 07, 2007 at 04:03:17PM -0700, Eric W. Biederman wrote: > I think the right answer is most likely to add an extra file method or > two so we can remove the need for is_file_hugepages. > There are still 4 calls to is_file_hugepages in ipc/shm.c and > 2 calls in mm/mmap.c not counting the one in is_file_shm_hugepages. > The special cases make it difficult to properly wrap hugetlbfs files > with another file, which is why we have the weird special case above. It's not clear to me that the core can be insulated from hugetlb's distinct pagecache and memory mapping granularities in a Linux-native manner, but if you come up with something new or manage to get the known methods past Linus, akpm, et al, more power to you. I'm not entirely sure what you're up to, but I'm mostly here to sanction others' design notions since my own are far too extreme, and, of course, review and ack patches, take bugreports and write fixes (not that I've managed to get to any of them first in a long while, if ever), and so on. I say killing the is_whatever_hugepages() checks with whatever abstraction is good, since I don't like them myself, provided it's sane. Go for it. -- wli - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
On Wed, 2007-03-07 at 16:03 -0700, Eric W. Biederman wrote: > Bill Irwin <[EMAIL PROTECTED]> writes: > > > On Thu, Mar 01, 2007 at 03:46:08PM -0800, Adam Litke wrote: > >> static inline int is_file_hugepages(struct file *file) > >> { > >> - return file->f_op == _file_operations; > >> + if (file->f_op == _file_operations) > >> + return 1; > >> + if (is_file_shm_hugepages(file)) > >> + return 1; > >> + > >> + return 0; > >> } > > ... > >> +int is_file_shm_hugepages(struct file *file) > >> +{ > >> + int ret = 0; > >> + > >> + if (file->f_op == _file_operations) { > >> + struct shm_file_data *sfd; > >> + sfd = shm_file_data(file); > >> + ret = is_file_hugepages(sfd->file); > >> + } > >> + return ret; > > > > A comment to prepare others for the impending doubletake might be nice. > > Or maybe just open-coding the equality check for _file_operations > > in is_file_shm_hugepages() if others find it as jarring as I. Please > > extend my ack to any follow-up fiddling with that. > > You did notice we are testing a different struct file? > > > The patch addresses relatively straightforward issues and naturally at > > that. > > The whole concept is recursive so I'm not certain being a recursive check > is that bad but I understand the point. > > I think the right answer is most likely to add an extra file method or > two so we can remove the need for is_file_hugepages. > > There are still 4 calls to is_file_hugepages in ipc/shm.c and > 2 calls in mm/mmap.c not counting the one in is_file_shm_hugepages. > > The special cases make it difficult to properly wrap hugetlbfs files > with another file, which is why we have the weird special case above. :) Enter my remove-is_file_hugepages() patches (which I posted a few weeks ago). I'll rework them and repost soon. That should help to make all of this cleaner. -- Adam Litke - (agl at us.ibm.com) IBM Linux Technology Center - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
Bill Irwin <[EMAIL PROTECTED]> writes: > On Thu, Mar 01, 2007 at 03:46:08PM -0800, Adam Litke wrote: >> static inline int is_file_hugepages(struct file *file) >> { >> -return file->f_op == _file_operations; >> +if (file->f_op == _file_operations) >> +return 1; >> +if (is_file_shm_hugepages(file)) >> +return 1; >> + >> +return 0; >> } > ... >> +int is_file_shm_hugepages(struct file *file) >> +{ >> +int ret = 0; >> + >> +if (file->f_op == _file_operations) { >> +struct shm_file_data *sfd; >> +sfd = shm_file_data(file); >> +ret = is_file_hugepages(sfd->file); >> +} >> +return ret; > > A comment to prepare others for the impending doubletake might be nice. > Or maybe just open-coding the equality check for _file_operations > in is_file_shm_hugepages() if others find it as jarring as I. Please > extend my ack to any follow-up fiddling with that. You did notice we are testing a different struct file? > The patch addresses relatively straightforward issues and naturally at > that. The whole concept is recursive so I'm not certain being a recursive check is that bad but I understand the point. I think the right answer is most likely to add an extra file method or two so we can remove the need for is_file_hugepages. There are still 4 calls to is_file_hugepages in ipc/shm.c and 2 calls in mm/mmap.c not counting the one in is_file_shm_hugepages. The special cases make it difficult to properly wrap hugetlbfs files with another file, which is why we have the weird special case above. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
Bill Irwin [EMAIL PROTECTED] writes: On Thu, Mar 01, 2007 at 03:46:08PM -0800, Adam Litke wrote: static inline int is_file_hugepages(struct file *file) { -return file-f_op == hugetlbfs_file_operations; +if (file-f_op == hugetlbfs_file_operations) +return 1; +if (is_file_shm_hugepages(file)) +return 1; + +return 0; } ... +int is_file_shm_hugepages(struct file *file) +{ +int ret = 0; + +if (file-f_op == shm_file_operations) { +struct shm_file_data *sfd; +sfd = shm_file_data(file); +ret = is_file_hugepages(sfd-file); +} +return ret; A comment to prepare others for the impending doubletake might be nice. Or maybe just open-coding the equality check for huetlbfs_file_operations in is_file_shm_hugepages() if others find it as jarring as I. Please extend my ack to any follow-up fiddling with that. You did notice we are testing a different struct file? The patch addresses relatively straightforward issues and naturally at that. The whole concept is recursive so I'm not certain being a recursive check is that bad but I understand the point. I think the right answer is most likely to add an extra file method or two so we can remove the need for is_file_hugepages. There are still 4 calls to is_file_hugepages in ipc/shm.c and 2 calls in mm/mmap.c not counting the one in is_file_shm_hugepages. The special cases make it difficult to properly wrap hugetlbfs files with another file, which is why we have the weird special case above. Eric - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
On Wed, 2007-03-07 at 16:03 -0700, Eric W. Biederman wrote: Bill Irwin [EMAIL PROTECTED] writes: On Thu, Mar 01, 2007 at 03:46:08PM -0800, Adam Litke wrote: static inline int is_file_hugepages(struct file *file) { - return file-f_op == hugetlbfs_file_operations; + if (file-f_op == hugetlbfs_file_operations) + return 1; + if (is_file_shm_hugepages(file)) + return 1; + + return 0; } ... +int is_file_shm_hugepages(struct file *file) +{ + int ret = 0; + + if (file-f_op == shm_file_operations) { + struct shm_file_data *sfd; + sfd = shm_file_data(file); + ret = is_file_hugepages(sfd-file); + } + return ret; A comment to prepare others for the impending doubletake might be nice. Or maybe just open-coding the equality check for huetlbfs_file_operations in is_file_shm_hugepages() if others find it as jarring as I. Please extend my ack to any follow-up fiddling with that. You did notice we are testing a different struct file? The patch addresses relatively straightforward issues and naturally at that. The whole concept is recursive so I'm not certain being a recursive check is that bad but I understand the point. I think the right answer is most likely to add an extra file method or two so we can remove the need for is_file_hugepages. There are still 4 calls to is_file_hugepages in ipc/shm.c and 2 calls in mm/mmap.c not counting the one in is_file_shm_hugepages. The special cases make it difficult to properly wrap hugetlbfs files with another file, which is why we have the weird special case above. :) Enter my remove-is_file_hugepages() patches (which I posted a few weeks ago). I'll rework them and repost soon. That should help to make all of this cleaner. -- Adam Litke - (agl at us.ibm.com) IBM Linux Technology Center - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
Bill Irwin [EMAIL PROTECTED] writes: A comment to prepare others for the impending doubletake might be nice. Or maybe just open-coding the equality check for hugetlbfs_file_operations in is_file_shm_hugepages() if others find it as jarring as I. Please extend my ack to any follow-up fiddling with that. On Wed, Mar 07, 2007 at 04:03:17PM -0700, Eric W. Biederman wrote: You did notice we are testing a different struct file? Yes. Bill Irwin [EMAIL PROTECTED] writes: The patch addresses relatively straightforward issues and naturally at that. On Wed, Mar 07, 2007 at 04:03:17PM -0700, Eric W. Biederman wrote: The whole concept is recursive so I'm not certain being a recursive check is that bad but I understand the point. Hence my characterization as natural. You did notice this was an ack? On Wed, Mar 07, 2007 at 04:03:17PM -0700, Eric W. Biederman wrote: I think the right answer is most likely to add an extra file method or two so we can remove the need for is_file_hugepages. There are still 4 calls to is_file_hugepages in ipc/shm.c and 2 calls in mm/mmap.c not counting the one in is_file_shm_hugepages. The special cases make it difficult to properly wrap hugetlbfs files with another file, which is why we have the weird special case above. It's not clear to me that the core can be insulated from hugetlb's distinct pagecache and memory mapping granularities in a Linux-native manner, but if you come up with something new or manage to get the known methods past Linus, akpm, et al, more power to you. I'm not entirely sure what you're up to, but I'm mostly here to sanction others' design notions since my own are far too extreme, and, of course, review and ack patches, take bugreports and write fixes (not that I've managed to get to any of them first in a long while, if ever), and so on. I say killing the is_whatever_hugepages() checks with whatever abstraction is good, since I don't like them myself, provided it's sane. Go for it. -- wli - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
On Wed, Mar 07, 2007 at 05:26:48PM -0600, Adam Litke wrote: :) Enter my remove-is_file_hugepages() patches (which I posted a few weeks ago). I'll rework them and repost soon. That should help to make all of this cleaner. Those were great. I've wanted something like them for a long, long time. -- wli - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
Bill Irwin [EMAIL PROTECTED] writes: On Wed, Mar 07, 2007 at 04:03:17PM -0700, Eric W. Biederman wrote: I think the right answer is most likely to add an extra file method or two so we can remove the need for is_file_hugepages. There are still 4 calls to is_file_hugepages in ipc/shm.c and 2 calls in mm/mmap.c not counting the one in is_file_shm_hugepages. The special cases make it difficult to properly wrap hugetlbfs files with another file, which is why we have the weird special case above. It's not clear to me that the core can be insulated from hugetlb's distinct pagecache and memory mapping granularities in a Linux-native manner, but if you come up with something new or manage to get the known methods past Linus, akpm, et al, more power to you. I will agree with that there are limits on what can be achieved. However looking at where we have tests for is_file_hugepages most of those tests don't appear to be inherently anything to do with huge pages, so it wouldn't surprise me if we couldn't generalize things a little more. I'm not entirely sure what you're up to, but I'm mostly here to sanction others' design notions since my own are far too extreme, and, of course, review and ack patches, take bugreports and write fixes (not that I've managed to get to any of them first in a long while, if ever), and so on. I say killing the is_whatever_hugepages() checks with whatever abstraction is good, since I don't like them myself, provided it's sane. Go for it. Mostly I had reference counting and consistency problems with ipc/shm.c that had horrible leak potential when I exited a ipc namespace. Implementing everything as stacked files made the code simpler and more maintainable. (shm_nattach stopped being a special case yea!) I'm happy to stop here but if someone cares to proceed with removing is_file_hugepages I want to encourage that. I don't see any other cleanups short of that are really worth doing. Everything in ipc/shm.c could be considered a weird special case, so I'm not going to worry about it too much. Although removing those special cases is good. There is some odd accounting logic in mm/mmap.c based on is_file_hugepages and there is the get_unmapped_area case. For get_unmapped_area I see no reason to presume that the only kind of file that must live at a specific address are huge pages (even if that is the only kind of file where we have that case today). So generalizing that check should be relatively straight forward. Eric - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
On Thu, Mar 01, 2007 at 03:46:08PM -0800, Adam Litke wrote: > static inline int is_file_hugepages(struct file *file) > { > - return file->f_op == _file_operations; > + if (file->f_op == _file_operations) > + return 1; > + if (is_file_shm_hugepages(file)) > + return 1; > + > + return 0; > } ... > +int is_file_shm_hugepages(struct file *file) > +{ > + int ret = 0; > + > + if (file->f_op == _file_operations) { > + struct shm_file_data *sfd; > + sfd = shm_file_data(file); > + ret = is_file_hugepages(sfd->file); > + } > + return ret; A comment to prepare others for the impending doubletake might be nice. Or maybe just open-coding the equality check for _file_operations in is_file_shm_hugepages() if others find it as jarring as I. Please extend my ack to any follow-up fiddling with that. The patch addresses relatively straightforward issues and naturally at that. Acked-by: William Irwin <[EMAIL PROTECTED]> -- wli - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
This patch provides the following hugetlb-related fixes to the recent stacked shm files changes: - Update is_file_hugepages() so it will reconize hugetlb shm segments. - get_unmapped_area must be called with the nested file struct to handle the sfd->file->f_ops->get_unmapped_area == NULL case. - The fsync f_op must be wrapped since it is specified in the hugetlbfs f_ops. This is based on proposed fixes from Eric Biederman that were debugged and tested by me. Without it, attempting to use hugetlb shared memory segments on powerpc (and likely ia64) will kill your box. Please Apply. Signed-off-by: Adam Litke <[EMAIL PROTECTED]> --- include/linux/hugetlb.h |8 +++- include/linux/shm.h |5 + ipc/shm.c | 32 ++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index a60995a..3f3e7a6 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h @@ -4,6 +4,7 @@ #ifdef CONFIG_HUGETLB_PAGE #include +#include #include struct ctl_table; @@ -168,7 +169,12 @@ void hugetlb_put_quota(struct address_space *mapping); static inline int is_file_hugepages(struct file *file) { - return file->f_op == _file_operations; + if (file->f_op == _file_operations) + return 1; + if (is_file_shm_hugepages(file)) + return 1; + + return 0; } static inline void set_file_hugepages(struct file *file) diff --git a/include/linux/shm.h b/include/linux/shm.h index a2c896a..ad2e3af 100644 --- a/include/linux/shm.h +++ b/include/linux/shm.h @@ -96,12 +96,17 @@ struct shmid_kernel /* private to the kernel */ #ifdef CONFIG_SYSVIPC long do_shmat(int shmid, char __user *shmaddr, int shmflg, unsigned long *addr); +extern int is_file_shm_hugepages(struct file *file); #else static inline long do_shmat(int shmid, char __user *shmaddr, int shmflg, unsigned long *addr) { return -ENOSYS; } +static inline int is_file_shm_hugepages(struct file *file) +{ + return 0; +} #endif #endif /* __KERNEL__ */ diff --git a/ipc/shm.c b/ipc/shm.c index eb57e22..abf864d 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -285,21 +285,41 @@ static int shm_release(struct inode *ino, struct file *file) return 0; } -#ifndef CONFIG_MMU +static int shm_fsync(struct file *file, struct dentry *dentry, int datasync) +{ + int (*fsync) (struct file *, struct dentry *, int datasync); + struct shm_file_data *sfd = shm_file_data(file); + int ret = -EINVAL; + + fsync = sfd->file->f_op->fsync; + if (fsync) + ret = fsync(sfd->file, sfd->file->f_path.dentry, datasync); + return ret; +} + static unsigned long shm_get_unmapped_area(struct file *file, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags) { struct shm_file_data *sfd = shm_file_data(file); - return sfd->file->f_op->get_unmapped_area(sfd->file, addr, len, pgoff, - flags); + return get_unmapped_area(sfd->file, addr, len, pgoff, flags); +} + +int is_file_shm_hugepages(struct file *file) +{ + int ret = 0; + + if (file->f_op == _file_operations) { + struct shm_file_data *sfd; + sfd = shm_file_data(file); + ret = is_file_hugepages(sfd->file); + } + return ret; } -#else -#define shm_get_unmapped_area NULL -#endif static const struct file_operations shm_file_operations = { .mmap = shm_mmap, + .fsync = shm_fsync, .release= shm_release, .get_unmapped_area = shm_get_unmapped_area, }; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
This patch provides the following hugetlb-related fixes to the recent stacked shm files changes: - Update is_file_hugepages() so it will reconize hugetlb shm segments. - get_unmapped_area must be called with the nested file struct to handle the sfd-file-f_ops-get_unmapped_area == NULL case. - The fsync f_op must be wrapped since it is specified in the hugetlbfs f_ops. This is based on proposed fixes from Eric Biederman that were debugged and tested by me. Without it, attempting to use hugetlb shared memory segments on powerpc (and likely ia64) will kill your box. Please Apply. Signed-off-by: Adam Litke [EMAIL PROTECTED] --- include/linux/hugetlb.h |8 +++- include/linux/shm.h |5 + ipc/shm.c | 32 ++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index a60995a..3f3e7a6 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h @@ -4,6 +4,7 @@ #ifdef CONFIG_HUGETLB_PAGE #include linux/mempolicy.h +#include linux/shm.h #include asm/tlbflush.h struct ctl_table; @@ -168,7 +169,12 @@ void hugetlb_put_quota(struct address_space *mapping); static inline int is_file_hugepages(struct file *file) { - return file-f_op == hugetlbfs_file_operations; + if (file-f_op == hugetlbfs_file_operations) + return 1; + if (is_file_shm_hugepages(file)) + return 1; + + return 0; } static inline void set_file_hugepages(struct file *file) diff --git a/include/linux/shm.h b/include/linux/shm.h index a2c896a..ad2e3af 100644 --- a/include/linux/shm.h +++ b/include/linux/shm.h @@ -96,12 +96,17 @@ struct shmid_kernel /* private to the kernel */ #ifdef CONFIG_SYSVIPC long do_shmat(int shmid, char __user *shmaddr, int shmflg, unsigned long *addr); +extern int is_file_shm_hugepages(struct file *file); #else static inline long do_shmat(int shmid, char __user *shmaddr, int shmflg, unsigned long *addr) { return -ENOSYS; } +static inline int is_file_shm_hugepages(struct file *file) +{ + return 0; +} #endif #endif /* __KERNEL__ */ diff --git a/ipc/shm.c b/ipc/shm.c index eb57e22..abf864d 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -285,21 +285,41 @@ static int shm_release(struct inode *ino, struct file *file) return 0; } -#ifndef CONFIG_MMU +static int shm_fsync(struct file *file, struct dentry *dentry, int datasync) +{ + int (*fsync) (struct file *, struct dentry *, int datasync); + struct shm_file_data *sfd = shm_file_data(file); + int ret = -EINVAL; + + fsync = sfd-file-f_op-fsync; + if (fsync) + ret = fsync(sfd-file, sfd-file-f_path.dentry, datasync); + return ret; +} + static unsigned long shm_get_unmapped_area(struct file *file, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags) { struct shm_file_data *sfd = shm_file_data(file); - return sfd-file-f_op-get_unmapped_area(sfd-file, addr, len, pgoff, - flags); + return get_unmapped_area(sfd-file, addr, len, pgoff, flags); +} + +int is_file_shm_hugepages(struct file *file) +{ + int ret = 0; + + if (file-f_op == shm_file_operations) { + struct shm_file_data *sfd; + sfd = shm_file_data(file); + ret = is_file_hugepages(sfd-file); + } + return ret; } -#else -#define shm_get_unmapped_area NULL -#endif static const struct file_operations shm_file_operations = { .mmap = shm_mmap, + .fsync = shm_fsync, .release= shm_release, .get_unmapped_area = shm_get_unmapped_area, }; - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments
On Thu, Mar 01, 2007 at 03:46:08PM -0800, Adam Litke wrote: static inline int is_file_hugepages(struct file *file) { - return file-f_op == hugetlbfs_file_operations; + if (file-f_op == hugetlbfs_file_operations) + return 1; + if (is_file_shm_hugepages(file)) + return 1; + + return 0; } ... +int is_file_shm_hugepages(struct file *file) +{ + int ret = 0; + + if (file-f_op == shm_file_operations) { + struct shm_file_data *sfd; + sfd = shm_file_data(file); + ret = is_file_hugepages(sfd-file); + } + return ret; A comment to prepare others for the impending doubletake might be nice. Or maybe just open-coding the equality check for huetlbfs_file_operations in is_file_shm_hugepages() if others find it as jarring as I. Please extend my ack to any follow-up fiddling with that. The patch addresses relatively straightforward issues and naturally at that. Acked-by: William Irwin [EMAIL PROTECTED] -- wli - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/