* max_user_watches overflows on 16TB system. @ 2010-10-01 20:01 Robin Holt 2010-10-01 20:37 ` Davide Libenzi 0 siblings, 1 reply; 10+ messages in thread From: Robin Holt @ 2010-10-01 20:01 UTC (permalink / raw) To: Davide Libenzi, Eric W. Biederman, Pekka Enberg; +Cc: linux-kernel Following a boot of a 16TB system, we noticed that the max_user_watches sysctl was negative. Is there any downside to converting that to a static long and handling the fallout from the change? I believe that fallout includes changing the definition of epoll_watches over to an atomic_long_t as well. Alternatively should we just limit max_user_watches to INT_MAX? Thanks, Robin Holt ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: max_user_watches overflows on 16TB system. 2010-10-01 20:01 max_user_watches overflows on 16TB system Robin Holt @ 2010-10-01 20:37 ` Davide Libenzi 2010-10-02 3:04 ` Eric W. Biederman 0 siblings, 1 reply; 10+ messages in thread From: Davide Libenzi @ 2010-10-01 20:37 UTC (permalink / raw) To: Robin Holt; +Cc: Eric W. Biederman, Pekka Enberg, linux-kernel On Fri, 1 Oct 2010, Robin Holt wrote: > > Following a boot of a 16TB system, we noticed that the max_user_watches > sysctl was negative. Is there any downside to converting that to a > static long and handling the fallout from the change? I believe that > fallout includes changing the definition of epoll_watches over to an > atomic_long_t as well. > > Alternatively should we just limit max_user_watches to INT_MAX? 2B watches looks an acceptable limit to me, at least for now. Nobody complained about not having enough of them so far. - Davide ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: max_user_watches overflows on 16TB system. 2010-10-01 20:37 ` Davide Libenzi @ 2010-10-02 3:04 ` Eric W. Biederman 2010-10-02 14:04 ` Davide Libenzi 0 siblings, 1 reply; 10+ messages in thread From: Eric W. Biederman @ 2010-10-02 3:04 UTC (permalink / raw) To: Davide Libenzi; +Cc: Robin Holt, Pekka Enberg, linux-kernel Davide Libenzi <davidel@xmailserver.org> writes: > On Fri, 1 Oct 2010, Robin Holt wrote: > >> >> Following a boot of a 16TB system, we noticed that the max_user_watches >> sysctl was negative. Is there any downside to converting that to a >> static long and handling the fallout from the change? I believe that >> fallout includes changing the definition of epoll_watches over to an >> atomic_long_t as well. >> >> Alternatively should we just limit max_user_watches to INT_MAX? > > 2B watches looks an acceptable limit to me, at least for now. > Nobody complained about not having enough of them so far. Which suggests that we need to force the boot time calculation to not exceed 2B. >From the sysctl interface perspective now that all of it is exported as ascii strings I don't see a problem there. Eric ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: max_user_watches overflows on 16TB system. 2010-10-02 3:04 ` Eric W. Biederman @ 2010-10-02 14:04 ` Davide Libenzi 2010-10-04 19:44 ` [Patch] Convert max_user_watches to long Robin Holt 0 siblings, 1 reply; 10+ messages in thread From: Davide Libenzi @ 2010-10-02 14:04 UTC (permalink / raw) To: Eric W. Biederman; +Cc: Robin Holt, Pekka Enberg, Linux Kernel Mailing List On Fri, 1 Oct 2010, Eric W. Biederman wrote: > Davide Libenzi <davidel@xmailserver.org> writes: > > > On Fri, 1 Oct 2010, Robin Holt wrote: > > > >> > >> Following a boot of a 16TB system, we noticed that the max_user_watches > >> sysctl was negative. Is there any downside to converting that to a > >> static long and handling the fallout from the change? I believe that > >> fallout includes changing the definition of epoll_watches over to an > >> atomic_long_t as well. > >> > >> Alternatively should we just limit max_user_watches to INT_MAX? > > > > 2B watches looks an acceptable limit to me, at least for now. > > Nobody complained about not having enough of them so far. > > Which suggests that we need to force the boot time calculation to not > exceed 2B. > > From the sysctl interface perspective now that all of it is exported as > ascii strings I don't see a problem there. Thinking with actual code at hands, you're right. It just makes more sense the 'long' variable conversion, even though MAX_INT watches are a huge number today. Robin, sorry for the confusion. Can you post the 'long' conversion patch? - Davide ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Patch] Convert max_user_watches to long. 2010-10-02 14:04 ` Davide Libenzi @ 2010-10-04 19:44 ` Robin Holt 2010-10-06 2:21 ` Davide Libenzi 0 siblings, 1 reply; 10+ messages in thread From: Robin Holt @ 2010-10-04 19:44 UTC (permalink / raw) To: Davide Libenzi Cc: Eric W. Biederman, Robin Holt, Pekka Enberg, Linux Kernel Mailing List On a 16TB machine, max_user_watches has an integer overflow. Convert it to use a long and handle the associated fallout. Signed-off-by: Robin Holt <holt@sgi.com> To: "Eric W. Biederman" <ebiederm@xmission.com> To: Davide Libenzi <davidel@xmailserver.org> To: linux-kernel@vger.kernel.org To: Pekka Enberg <penberg@cs.helsinki.fi> --- Davide, I changed the logic a bit in ep_insert. It looked to me like there was a window between when the epoll_watches is checked and when it is incremented where multiple epoll_insert callers could be adding watches at the same time and allow epoll_watches to exceed max_user_watches. Not sure of the case where this could happen, but I assume something like that must be possible or we would not be using atomics. If that is not to your liking, I will happily remove it. fs/eventpoll.c | 22 +++++++++++++--------- include/linux/sched.h | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) Index: pv1010933/fs/eventpoll.c =================================================================== --- pv1010933.orig/fs/eventpoll.c 2010-10-02 06:38:15.000000000 -0500 +++ pv1010933/fs/eventpoll.c 2010-10-04 11:05:21.643823297 -0500 @@ -220,7 +220,7 @@ struct ep_send_events_data { * Configuration options available inside /proc/sys/fs/epoll/ */ /* Maximum number of epoll watched descriptors, per user */ -static int max_user_watches __read_mostly; +static long max_user_watches __read_mostly; /* * This mutex is used to serialize ep_free() and eventpoll_release_file(). @@ -243,16 +243,18 @@ static struct kmem_cache *pwq_cache __re #include <linux/sysctl.h> -static int zero; +static long zero; +static long long_max = LONG_MAX; ctl_table epoll_table[] = { { .procname = "max_user_watches", .data = &max_user_watches, - .maxlen = sizeof(int), + .maxlen = sizeof(max_user_watches), .mode = 0644, - .proc_handler = proc_dointvec_minmax, + .proc_handler = proc_doulongvec_minmax, .extra1 = &zero, + .extra2 = &long_max, }, { } }; @@ -564,7 +566,7 @@ static int ep_remove(struct eventpoll *e /* At this point it is safe to free the eventpoll item */ kmem_cache_free(epi_cache, epi); - atomic_dec(&ep->user->epoll_watches); + atomic_long_dec(&ep->user->epoll_watches); return 0; } @@ -900,12 +902,15 @@ static int ep_insert(struct eventpoll *e { int error, revents, pwake = 0; unsigned long flags; + long user_watches; struct epitem *epi; struct ep_pqueue epq; - if (unlikely(atomic_read(&ep->user->epoll_watches) >= - max_user_watches)) + user_watches = atomic_long_inc_return(&ep->user->epoll_watches); + if (unlikely(user_watches > max_user_watches)) { + atomic_long_dec(&ep->user->epoll_watches); return -ENOSPC; + } if (!(epi = kmem_cache_alloc(epi_cache, GFP_KERNEL))) return -ENOMEM; @@ -968,8 +973,6 @@ static int ep_insert(struct eventpoll *e spin_unlock_irqrestore(&ep->lock, flags); - atomic_inc(&ep->user->epoll_watches); - /* We have to call this outside the lock */ if (pwake) ep_poll_safewake(&ep->poll_wait); @@ -1422,6 +1425,7 @@ static int __init eventpoll_init(void) */ max_user_watches = (((si.totalram - si.totalhigh) / 25) << PAGE_SHIFT) / EP_ITEM_COST; + BUG_ON(max_user_watches < 0); /* Initialize the structure used to perform safe poll wait head wake ups */ ep_nested_calls_init(&poll_safewake_ncalls); Index: pv1010933/include/linux/sched.h =================================================================== --- pv1010933.orig/include/linux/sched.h 2010-10-01 10:27:07.000000000 -0500 +++ pv1010933/include/linux/sched.h 2010-10-04 10:44:11.287823312 -0500 @@ -666,7 +666,7 @@ struct user_struct { atomic_t inotify_devs; /* How many inotify devs does this user have opened? */ #endif #ifdef CONFIG_EPOLL - atomic_t epoll_watches; /* The number of file descriptors currently watched */ + atomic_long_t epoll_watches; /* The number of file descriptors currently watched */ #endif #ifdef CONFIG_POSIX_MQUEUE /* protected by mq_lock */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] Convert max_user_watches to long. 2010-10-04 19:44 ` [Patch] Convert max_user_watches to long Robin Holt @ 2010-10-06 2:21 ` Davide Libenzi 2010-10-09 7:50 ` Robin Holt 0 siblings, 1 reply; 10+ messages in thread From: Davide Libenzi @ 2010-10-06 2:21 UTC (permalink / raw) To: Robin Holt; +Cc: Eric W. Biederman, Pekka Enberg, Linux Kernel Mailing List On Mon, 4 Oct 2010, Robin Holt wrote: > On a 16TB machine, max_user_watches has an integer overflow. Convert it > to use a long and handle the associated fallout. > > > Signed-off-by: Robin Holt <holt@sgi.com> > To: "Eric W. Biederman" <ebiederm@xmission.com> > To: Davide Libenzi <davidel@xmailserver.org> > To: linux-kernel@vger.kernel.org > To: Pekka Enberg <penberg@cs.helsinki.fi> > > --- > > Davide, I changed the logic a bit in ep_insert. It looked to me like > there was a window between when the epoll_watches is checked and when it > is incremented where multiple epoll_insert callers could be adding watches > at the same time and allow epoll_watches to exceed max_user_watches. > Not sure of the case where this could happen, but I assume something > like that must be possible or we would not be using atomics. If that > is not to your liking, I will happily remove it. The case can happen, but the effect is not something we should be too worried about. You seem to be leaking a count in case kmem_cache_alloc() and following fail. I'd rather not have that code there, and have the patch cover the 'long' conversion only. Or you need a proper cleanup goto target. > > fs/eventpoll.c | 22 +++++++++++++--------- > include/linux/sched.h | 2 +- > 2 files changed, 14 insertions(+), 10 deletions(-) > > Index: pv1010933/fs/eventpoll.c > =================================================================== > --- pv1010933.orig/fs/eventpoll.c 2010-10-02 06:38:15.000000000 -0500 > +++ pv1010933/fs/eventpoll.c 2010-10-04 11:05:21.643823297 -0500 > @@ -220,7 +220,7 @@ struct ep_send_events_data { > * Configuration options available inside /proc/sys/fs/epoll/ > */ > /* Maximum number of epoll watched descriptors, per user */ > -static int max_user_watches __read_mostly; > +static long max_user_watches __read_mostly; > > /* > * This mutex is used to serialize ep_free() and eventpoll_release_file(). > @@ -243,16 +243,18 @@ static struct kmem_cache *pwq_cache __re > > #include <linux/sysctl.h> > > -static int zero; > +static long zero; > +static long long_max = LONG_MAX; > > ctl_table epoll_table[] = { > { > .procname = "max_user_watches", > .data = &max_user_watches, > - .maxlen = sizeof(int), > + .maxlen = sizeof(max_user_watches), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > + .proc_handler = proc_doulongvec_minmax, > .extra1 = &zero, > + .extra2 = &long_max, > }, > { } > }; > @@ -564,7 +566,7 @@ static int ep_remove(struct eventpoll *e > /* At this point it is safe to free the eventpoll item */ > kmem_cache_free(epi_cache, epi); > > - atomic_dec(&ep->user->epoll_watches); > + atomic_long_dec(&ep->user->epoll_watches); > > return 0; > } > @@ -900,12 +902,15 @@ static int ep_insert(struct eventpoll *e > { > int error, revents, pwake = 0; > unsigned long flags; > + long user_watches; > struct epitem *epi; > struct ep_pqueue epq; > > - if (unlikely(atomic_read(&ep->user->epoll_watches) >= > - max_user_watches)) > + user_watches = atomic_long_inc_return(&ep->user->epoll_watches); > + if (unlikely(user_watches > max_user_watches)) { > + atomic_long_dec(&ep->user->epoll_watches); > return -ENOSPC; > + } > if (!(epi = kmem_cache_alloc(epi_cache, GFP_KERNEL))) > return -ENOMEM; > > @@ -968,8 +973,6 @@ static int ep_insert(struct eventpoll *e > > spin_unlock_irqrestore(&ep->lock, flags); > > - atomic_inc(&ep->user->epoll_watches); > - > /* We have to call this outside the lock */ > if (pwake) > ep_poll_safewake(&ep->poll_wait); > @@ -1422,6 +1425,7 @@ static int __init eventpoll_init(void) > */ > max_user_watches = (((si.totalram - si.totalhigh) / 25) << PAGE_SHIFT) / > EP_ITEM_COST; > + BUG_ON(max_user_watches < 0); > > /* Initialize the structure used to perform safe poll wait head wake ups */ > ep_nested_calls_init(&poll_safewake_ncalls); > Index: pv1010933/include/linux/sched.h > =================================================================== > --- pv1010933.orig/include/linux/sched.h 2010-10-01 10:27:07.000000000 -0500 > +++ pv1010933/include/linux/sched.h 2010-10-04 10:44:11.287823312 -0500 > @@ -666,7 +666,7 @@ struct user_struct { > atomic_t inotify_devs; /* How many inotify devs does this user have opened? */ > #endif > #ifdef CONFIG_EPOLL > - atomic_t epoll_watches; /* The number of file descriptors currently watched */ > + atomic_long_t epoll_watches; /* The number of file descriptors currently watched */ > #endif > #ifdef CONFIG_POSIX_MQUEUE > /* protected by mq_lock */ > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] Convert max_user_watches to long. 2010-10-06 2:21 ` Davide Libenzi @ 2010-10-09 7:50 ` Robin Holt 2010-10-10 19:05 ` Randy Dunlap 2010-10-11 4:49 ` Davide Libenzi 0 siblings, 2 replies; 10+ messages in thread From: Robin Holt @ 2010-10-09 7:50 UTC (permalink / raw) To: Davide Libenzi Cc: Robin Holt, Eric W. Biederman, Pekka Enberg, Linux Kernel Mailing List On Tue, Oct 05, 2010 at 07:21:09PM -0700, Davide Libenzi wrote: > On Mon, 4 Oct 2010, Robin Holt wrote: > > > On a 16TB machine, max_user_watches has an integer overflow. Convert it > > to use a long and handle the associated fallout. > > > > > > Signed-off-by: Robin Holt <holt@sgi.com> > > To: "Eric W. Biederman" <ebiederm@xmission.com> > > To: Davide Libenzi <davidel@xmailserver.org> > > To: linux-kernel@vger.kernel.org > > To: Pekka Enberg <penberg@cs.helsinki.fi> > > > > --- > > > > Davide, I changed the logic a bit in ep_insert. It looked to me like > > there was a window between when the epoll_watches is checked and when it > > is incremented where multiple epoll_insert callers could be adding watches > > at the same time and allow epoll_watches to exceed max_user_watches. > > Not sure of the case where this could happen, but I assume something > > like that must be possible or we would not be using atomics. If that > > is not to your liking, I will happily remove it. > > The case can happen, but the effect is not something we should be too > worried about. > You seem to be leaking a count in case kmem_cache_alloc() and following > fail. > I'd rather not have that code there, and have the patch cover the 'long' > conversion only. Or you need a proper cleanup goto target. Bah. Too rushed when I made that. Here is the conversion only patch. If this is acceptable, what is the normal submission path for fs/eventpoll.c? Robin ------------------------------------------------------------------------ On a 16TB machine, max_user_watches has an integer overflow. Convert it to use a long and handle the associated fallout. Signed-off-by: Robin Holt <holt@sgi.com> To: "Eric W. Biederman" <ebiederm@xmission.com> To: Davide Libenzi <davidel@xmailserver.org> To: linux-kernel@vger.kernel.org To: Pekka Enberg <penberg@cs.helsinki.fi> --- fs/eventpoll.c | 20 ++++++++++++-------- include/linux/sched.h | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) Index: pv1010933/fs/eventpoll.c =================================================================== --- pv1010933.orig/fs/eventpoll.c 2010-10-04 14:41:59.000000000 -0500 +++ pv1010933/fs/eventpoll.c 2010-10-09 02:40:07.360573988 -0500 @@ -220,7 +220,7 @@ struct ep_send_events_data { * Configuration options available inside /proc/sys/fs/epoll/ */ /* Maximum number of epoll watched descriptors, per user */ -static int max_user_watches __read_mostly; +static long max_user_watches __read_mostly; /* * This mutex is used to serialize ep_free() and eventpoll_release_file(). @@ -243,16 +243,18 @@ static struct kmem_cache *pwq_cache __re #include <linux/sysctl.h> -static int zero; +static long zero; +static long long_max = LONG_MAX; ctl_table epoll_table[] = { { .procname = "max_user_watches", .data = &max_user_watches, - .maxlen = sizeof(int), + .maxlen = sizeof(max_user_watches), .mode = 0644, - .proc_handler = proc_dointvec_minmax, + .proc_handler = proc_doulongvec_minmax, .extra1 = &zero, + .extra2 = &long_max, }, { } }; @@ -564,7 +566,7 @@ static int ep_remove(struct eventpoll *e /* At this point it is safe to free the eventpoll item */ kmem_cache_free(epi_cache, epi); - atomic_dec(&ep->user->epoll_watches); + atomic_long_dec(&ep->user->epoll_watches); return 0; } @@ -900,11 +902,12 @@ static int ep_insert(struct eventpoll *e { int error, revents, pwake = 0; unsigned long flags; + long user_watches; struct epitem *epi; struct ep_pqueue epq; - if (unlikely(atomic_read(&ep->user->epoll_watches) >= - max_user_watches)) + user_watches = atomic_long_read(&ep->user->epoll_watches); + if (user_watches >= max_user_watches) return -ENOSPC; if (!(epi = kmem_cache_alloc(epi_cache, GFP_KERNEL))) return -ENOMEM; @@ -968,7 +971,7 @@ static int ep_insert(struct eventpoll *e spin_unlock_irqrestore(&ep->lock, flags); - atomic_inc(&ep->user->epoll_watches); + atomic_long_inc(&ep->user->epoll_watches); /* We have to call this outside the lock */ if (pwake) @@ -1422,6 +1425,7 @@ static int __init eventpoll_init(void) */ max_user_watches = (((si.totalram - si.totalhigh) / 25) << PAGE_SHIFT) / EP_ITEM_COST; + BUG_ON(max_user_watches < 0); /* Initialize the structure used to perform safe poll wait head wake ups */ ep_nested_calls_init(&poll_safewake_ncalls); Index: pv1010933/include/linux/sched.h =================================================================== --- pv1010933.orig/include/linux/sched.h 2010-10-04 14:41:59.000000000 -0500 +++ pv1010933/include/linux/sched.h 2010-10-04 14:42:01.123824797 -0500 @@ -666,7 +666,7 @@ struct user_struct { atomic_t inotify_devs; /* How many inotify devs does this user have opened? */ #endif #ifdef CONFIG_EPOLL - atomic_t epoll_watches; /* The number of file descriptors currently watched */ + atomic_long_t epoll_watches; /* The number of file descriptors currently watched */ #endif #ifdef CONFIG_POSIX_MQUEUE /* protected by mq_lock */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] Convert max_user_watches to long. 2010-10-09 7:50 ` Robin Holt @ 2010-10-10 19:05 ` Randy Dunlap 2010-10-11 4:49 ` Davide Libenzi 1 sibling, 0 replies; 10+ messages in thread From: Randy Dunlap @ 2010-10-10 19:05 UTC (permalink / raw) To: Robin Holt, akpm Cc: Davide Libenzi, Eric W. Biederman, Pekka Enberg, Linux Kernel Mailing List On Sat, 9 Oct 2010 02:50:02 -0500 Robin Holt wrote: > On Tue, Oct 05, 2010 at 07:21:09PM -0700, Davide Libenzi wrote: > > On Mon, 4 Oct 2010, Robin Holt wrote: > > > > > On a 16TB machine, max_user_watches has an integer overflow. Convert it > > > to use a long and handle the associated fallout. > > > > > > > > > Signed-off-by: Robin Holt <holt@sgi.com> > > > To: "Eric W. Biederman" <ebiederm@xmission.com> > > > To: Davide Libenzi <davidel@xmailserver.org> > > > To: linux-kernel@vger.kernel.org > > > To: Pekka Enberg <penberg@cs.helsinki.fi> > > > > > > --- > > > > > > Davide, I changed the logic a bit in ep_insert. It looked to me like > > > there was a window between when the epoll_watches is checked and when it > > > is incremented where multiple epoll_insert callers could be adding watches > > > at the same time and allow epoll_watches to exceed max_user_watches. > > > Not sure of the case where this could happen, but I assume something > > > like that must be possible or we would not be using atomics. If that > > > is not to your liking, I will happily remove it. > > > > The case can happen, but the effect is not something we should be too > > worried about. > > You seem to be leaking a count in case kmem_cache_alloc() and following > > fail. > > I'd rather not have that code there, and have the patch cover the 'long' > > conversion only. Or you need a proper cleanup goto target. > > Bah. Too rushed when I made that. Here is the conversion only patch. If > this is acceptable, what is the normal submission path for fs/eventpoll.c? git log looks mostly like either thru Andrew or straight to Linus. > Robin > > ------------------------------------------------------------------------ > > On a 16TB machine, max_user_watches has an integer overflow. Convert it > to use a long and handle the associated fallout. > > > Signed-off-by: Robin Holt <holt@sgi.com> > To: "Eric W. Biederman" <ebiederm@xmission.com> > To: Davide Libenzi <davidel@xmailserver.org> > To: linux-kernel@vger.kernel.org > To: Pekka Enberg <penberg@cs.helsinki.fi> > > --- > > fs/eventpoll.c | 20 ++++++++++++-------- > include/linux/sched.h | 2 +- > 2 files changed, 13 insertions(+), 9 deletions(-) > > Index: pv1010933/fs/eventpoll.c > =================================================================== > --- pv1010933.orig/fs/eventpoll.c 2010-10-04 14:41:59.000000000 -0500 > +++ pv1010933/fs/eventpoll.c 2010-10-09 02:40:07.360573988 -0500 > @@ -220,7 +220,7 @@ struct ep_send_events_data { > * Configuration options available inside /proc/sys/fs/epoll/ > */ > /* Maximum number of epoll watched descriptors, per user */ > -static int max_user_watches __read_mostly; > +static long max_user_watches __read_mostly; > > /* > * This mutex is used to serialize ep_free() and eventpoll_release_file(). > @@ -243,16 +243,18 @@ static struct kmem_cache *pwq_cache __re > > #include <linux/sysctl.h> > > -static int zero; > +static long zero; > +static long long_max = LONG_MAX; > > ctl_table epoll_table[] = { > { > .procname = "max_user_watches", > .data = &max_user_watches, > - .maxlen = sizeof(int), > + .maxlen = sizeof(max_user_watches), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > + .proc_handler = proc_doulongvec_minmax, > .extra1 = &zero, > + .extra2 = &long_max, > }, > { } > }; > @@ -564,7 +566,7 @@ static int ep_remove(struct eventpoll *e > /* At this point it is safe to free the eventpoll item */ > kmem_cache_free(epi_cache, epi); > > - atomic_dec(&ep->user->epoll_watches); > + atomic_long_dec(&ep->user->epoll_watches); > > return 0; > } > @@ -900,11 +902,12 @@ static int ep_insert(struct eventpoll *e > { > int error, revents, pwake = 0; > unsigned long flags; > + long user_watches; > struct epitem *epi; > struct ep_pqueue epq; > > - if (unlikely(atomic_read(&ep->user->epoll_watches) >= > - max_user_watches)) > + user_watches = atomic_long_read(&ep->user->epoll_watches); > + if (user_watches >= max_user_watches) > return -ENOSPC; > if (!(epi = kmem_cache_alloc(epi_cache, GFP_KERNEL))) > return -ENOMEM; > @@ -968,7 +971,7 @@ static int ep_insert(struct eventpoll *e > > spin_unlock_irqrestore(&ep->lock, flags); > > - atomic_inc(&ep->user->epoll_watches); > + atomic_long_inc(&ep->user->epoll_watches); > > /* We have to call this outside the lock */ > if (pwake) > @@ -1422,6 +1425,7 @@ static int __init eventpoll_init(void) > */ > max_user_watches = (((si.totalram - si.totalhigh) / 25) << PAGE_SHIFT) / > EP_ITEM_COST; > + BUG_ON(max_user_watches < 0); > > /* Initialize the structure used to perform safe poll wait head wake ups */ > ep_nested_calls_init(&poll_safewake_ncalls); > Index: pv1010933/include/linux/sched.h > =================================================================== > --- pv1010933.orig/include/linux/sched.h 2010-10-04 14:41:59.000000000 -0500 > +++ pv1010933/include/linux/sched.h 2010-10-04 14:42:01.123824797 -0500 > @@ -666,7 +666,7 @@ struct user_struct { > atomic_t inotify_devs; /* How many inotify devs does this user have opened? */ > #endif > #ifdef CONFIG_EPOLL > - atomic_t epoll_watches; /* The number of file descriptors currently watched */ > + atomic_long_t epoll_watches; /* The number of file descriptors currently watched */ > #endif > #ifdef CONFIG_POSIX_MQUEUE > /* protected by mq_lock */ > -- --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] Convert max_user_watches to long. 2010-10-09 7:50 ` Robin Holt 2010-10-10 19:05 ` Randy Dunlap @ 2010-10-11 4:49 ` Davide Libenzi 2010-10-14 17:15 ` Robin Holt 1 sibling, 1 reply; 10+ messages in thread From: Davide Libenzi @ 2010-10-11 4:49 UTC (permalink / raw) To: Robin Holt; +Cc: Eric W. Biederman, Pekka Enberg, Linux Kernel Mailing List On Sat, 9 Oct 2010, Robin Holt wrote: > @@ -900,11 +902,12 @@ static int ep_insert(struct eventpoll *e > { > int error, revents, pwake = 0; > unsigned long flags; > + long user_watches; > struct epitem *epi; > struct ep_pqueue epq; > > - if (unlikely(atomic_read(&ep->user->epoll_watches) >= > - max_user_watches)) > + user_watches = atomic_long_read(&ep->user->epoll_watches); > + if (user_watches >= max_user_watches) > return -ENOSPC; Is there a particular reason for adding an extra, otherwise unused, user_watches variable? - Davide ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch] Convert max_user_watches to long. 2010-10-11 4:49 ` Davide Libenzi @ 2010-10-14 17:15 ` Robin Holt 0 siblings, 0 replies; 10+ messages in thread From: Robin Holt @ 2010-10-14 17:15 UTC (permalink / raw) To: Davide Libenzi Cc: Robin Holt, Eric W. Biederman, Pekka Enberg, Linux Kernel Mailing List On Sun, Oct 10, 2010 at 09:49:58PM -0700, Davide Libenzi wrote: > On Sat, 9 Oct 2010, Robin Holt wrote: > > > @@ -900,11 +902,12 @@ static int ep_insert(struct eventpoll *e > > { > > int error, revents, pwake = 0; > > unsigned long flags; > > + long user_watches; > > struct epitem *epi; > > struct ep_pqueue epq; > > > > - if (unlikely(atomic_read(&ep->user->epoll_watches) >= > > - max_user_watches)) > > + user_watches = atomic_long_read(&ep->user->epoll_watches); > > + if (user_watches >= max_user_watches) > > return -ENOSPC; > > Is there a particular reason for adding an extra, otherwise unused, > user_watches variable? Keeps the line length and readability complaints down. No other reason. Robin ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-10-14 17:15 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-01 20:01 max_user_watches overflows on 16TB system Robin Holt 2010-10-01 20:37 ` Davide Libenzi 2010-10-02 3:04 ` Eric W. Biederman 2010-10-02 14:04 ` Davide Libenzi 2010-10-04 19:44 ` [Patch] Convert max_user_watches to long Robin Holt 2010-10-06 2:21 ` Davide Libenzi 2010-10-09 7:50 ` Robin Holt 2010-10-10 19:05 ` Randy Dunlap 2010-10-11 4:49 ` Davide Libenzi 2010-10-14 17:15 ` Robin Holt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox