| Message ID | 55B5A04D.8090905@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 27 Jul 2015 11:06:53 +0800 Kinglong Mee <kinglongmee@gmail.com> wrote: > Without initialized, done in fs_pin at stack space may > contains strange value. > > v8, same as v3 > Adds macro for header file > > Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> Reviewed-by: NeilBrown <neilb@suse.com> It would be really good if some of these early patches could be applied to the relevant trees so they appear in -next and we only need to keep reviewing the more interesting code at the end. Al, Bruce: any chance of some of these getting into -next ... Thanks, NeilBrown > --- > include/linux/fs_pin.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/include/linux/fs_pin.h b/include/linux/fs_pin.h > index 3886b3b..0dde7b7 100644 > --- a/include/linux/fs_pin.h > +++ b/include/linux/fs_pin.h > @@ -1,3 +1,6 @@ > +#ifndef _LINUX_FS_PIN_H > +#define _LINUX_FS_PIN_H > + > #include <linux/wait.h> > > struct fs_pin { > @@ -16,9 +19,12 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *)) > INIT_HLIST_NODE(&p->s_list); > INIT_HLIST_NODE(&p->m_list); > p->kill = kill; > + p->done = 0; > } > > void pin_remove(struct fs_pin *); > void pin_insert_group(struct fs_pin *, struct vfsmount *, struct hlist_head *); > void pin_insert(struct fs_pin *, struct vfsmount *); > void pin_kill(struct fs_pin *); > + > +#endif -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 29, 2015 at 10:25:19AM +1000, NeilBrown wrote: > On Mon, 27 Jul 2015 11:06:53 +0800 Kinglong Mee <kinglongmee@gmail.com> > wrote: > > > Without initialized, done in fs_pin at stack space may > > contains strange value. > > > > v8, same as v3 > > Adds macro for header file > > > > Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> > > Reviewed-by: NeilBrown <neilb@suse.com> > > It would be really good if some of these early patches could be applied > to the relevant trees so they appear in -next and we only need to keep > reviewing the more interesting code at the end. This patch seems a little bikeshed-y. I'd rather just drop it or save it for some other day. It's not necessary to the series. --b. > > Al, Bruce: any chance of some of these getting into -next ... > > Thanks, > NeilBrown > > > --- > > include/linux/fs_pin.h | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/include/linux/fs_pin.h b/include/linux/fs_pin.h > > index 3886b3b..0dde7b7 100644 > > --- a/include/linux/fs_pin.h > > +++ b/include/linux/fs_pin.h > > @@ -1,3 +1,6 @@ > > +#ifndef _LINUX_FS_PIN_H > > +#define _LINUX_FS_PIN_H > > + > > #include <linux/wait.h> > > > > struct fs_pin { > > @@ -16,9 +19,12 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *)) > > INIT_HLIST_NODE(&p->s_list); > > INIT_HLIST_NODE(&p->m_list); > > p->kill = kill; > > + p->done = 0; > > } > > > > void pin_remove(struct fs_pin *); > > void pin_insert_group(struct fs_pin *, struct vfsmount *, struct hlist_head *); > > void pin_insert(struct fs_pin *, struct vfsmount *); > > void pin_kill(struct fs_pin *); > > + > > +#endif -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 29 Jul 2015 15:41:55 -0400 "J. Bruce Fields" <bfields@fieldses.org> wrote: > On Wed, Jul 29, 2015 at 10:25:19AM +1000, NeilBrown wrote: > > On Mon, 27 Jul 2015 11:06:53 +0800 Kinglong Mee <kinglongmee@gmail.com> > > wrote: > > > > > Without initialized, done in fs_pin at stack space may > > > contains strange value. > > > > > > v8, same as v3 > > > Adds macro for header file > > > > > > Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> > > > > Reviewed-by: NeilBrown <neilb@suse.com> > > > > It would be really good if some of these early patches could be applied > > to the relevant trees so they appear in -next and we only need to keep > > reviewing the more interesting code at the end. > > This patch seems a little bikeshed-y. I'd rather just drop it or save > it for some other day. It's not necessary to the series. ??? I accept that: > > > @@ -1,3 +1,6 @@ > > > +#ifndef _LINUX_FS_PIN_H > > > +#define _LINUX_FS_PIN_H > > > + > > > #include <linux/wait.h> could be a little bike-shed-y, not that I've seen much bike shedding going on. However: > > > > > > struct fs_pin { > > > @@ -16,9 +19,12 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *)) > > > INIT_HLIST_NODE(&p->s_list); > > > INIT_HLIST_NODE(&p->m_list); > > > p->kill = kill; > > > + p->done = 0; > > > } > > > is quite important. Without that assignment we would probably need to rename the function to init_most_of_fs_pin or init_fs_pin_if_already_zeroed or maybe just __init_fs_pin with the time honoured interpretation that it sort-of does what the name says, but maybe not exactly how you think and please use with care. Then in nfsd code we would need to add the assignment ourselves, or use kzalloc where it would otherwise be completely unnecessary. Thanks for accepting the other patches! NeilBrown -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 30, 2015 at 07:48:24AM +1000, NeilBrown wrote: > On Wed, 29 Jul 2015 15:41:55 -0400 "J. Bruce Fields" > <bfields@fieldses.org> wrote: > > > On Wed, Jul 29, 2015 at 10:25:19AM +1000, NeilBrown wrote: > > > On Mon, 27 Jul 2015 11:06:53 +0800 Kinglong Mee <kinglongmee@gmail.com> > > > wrote: > > > > > > > Without initialized, done in fs_pin at stack space may > > > > contains strange value. > > > > > > > > v8, same as v3 > > > > Adds macro for header file > > > > > > > > Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> > > > > > > Reviewed-by: NeilBrown <neilb@suse.com> > > > > > > It would be really good if some of these early patches could be applied > > > to the relevant trees so they appear in -next and we only need to keep > > > reviewing the more interesting code at the end. > > > > This patch seems a little bikeshed-y. I'd rather just drop it or save > > it for some other day. It's not necessary to the series. > > ??? > > I accept that: > > > > > > @@ -1,3 +1,6 @@ > > > > +#ifndef _LINUX_FS_PIN_H > > > > +#define _LINUX_FS_PIN_H > > > > + > > > > #include <linux/wait.h> > > could be a little bike-shed-y, not that I've seen much bike shedding > going on. > > However: > > > > > > > > struct fs_pin { > > > > @@ -16,9 +19,12 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *)) > > > > INIT_HLIST_NODE(&p->s_list); > > > > INIT_HLIST_NODE(&p->m_list); > > > > p->kill = kill; > > > > + p->done = 0; > > > > } > > > > > > is quite important. > Without that assignment we would probably need to rename the function to > init_most_of_fs_pin > or > init_fs_pin_if_already_zeroed > or maybe just > __init_fs_pin > with the time honoured interpretation that it sort-of does what the > name says, but maybe not exactly how you think and please use with care. > > Then in nfsd code we would need to add the assignment ourselves, or use > kzalloc where it would otherwise be completely unnecessary. Right, the existing users do something like kzalloc, last I checked. Maybe it'd be an improvement not to. I don't really care, but since Al's a bit overloaded, and you don't want to see this reposted, and it's not really essential to the series, why not drop it for now? --b. > > > Thanks for accepting the other patches! > > NeilBrown -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/30/2015 08:36, J. Bruce Fields wrote: > On Thu, Jul 30, 2015 at 07:48:24AM +1000, NeilBrown wrote: >> On Wed, 29 Jul 2015 15:41:55 -0400 "J. Bruce Fields" >> <bfields@fieldses.org> wrote: >> >>> On Wed, Jul 29, 2015 at 10:25:19AM +1000, NeilBrown wrote: >>>> On Mon, 27 Jul 2015 11:06:53 +0800 Kinglong Mee <kinglongmee@gmail.com> >>>> wrote: >>>> >>>>> Without initialized, done in fs_pin at stack space may >>>>> contains strange value. >>>>> >>>>> v8, same as v3 >>>>> Adds macro for header file >>>>> >>>>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> >>>> >>>> Reviewed-by: NeilBrown <neilb@suse.com> >>>> >>>> It would be really good if some of these early patches could be applied >>>> to the relevant trees so they appear in -next and we only need to keep >>>> reviewing the more interesting code at the end. >>> >>> This patch seems a little bikeshed-y. I'd rather just drop it or save >>> it for some other day. It's not necessary to the series. >> >> ??? >> >> I accept that: >> >> >>>>> @@ -1,3 +1,6 @@ >>>>> +#ifndef _LINUX_FS_PIN_H >>>>> +#define _LINUX_FS_PIN_H >>>>> + >>>>> #include <linux/wait.h> >> >> could be a little bike-shed-y, not that I've seen much bike shedding >> going on. >> >> However: >>>>> >>>>> struct fs_pin { >>>>> @@ -16,9 +19,12 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *)) >>>>> INIT_HLIST_NODE(&p->s_list); >>>>> INIT_HLIST_NODE(&p->m_list); >>>>> p->kill = kill; >>>>> + p->done = 0; >>>>> } >>>>> >> >> is quite important. >> Without that assignment we would probably need to rename the function to >> init_most_of_fs_pin >> or >> init_fs_pin_if_already_zeroed >> or maybe just >> __init_fs_pin >> with the time honoured interpretation that it sort-of does what the >> name says, but maybe not exactly how you think and please use with care. >> >> Then in nfsd code we would need to add the assignment ourselves, or use >> kzalloc where it would otherwise be completely unnecessary. > > Right, the existing users do something like kzalloc, last I checked. > Maybe it'd be an improvement not to. > > I don't really care, but since Al's a bit overloaded, and you don't want > to see this reposted, and it's not really essential to the series, why > not drop it for now? What's your opinion about this patch, Al? Drop? needs update? thanks, Kinglong Mee -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/fs_pin.h b/include/linux/fs_pin.h index 3886b3b..0dde7b7 100644 --- a/include/linux/fs_pin.h +++ b/include/linux/fs_pin.h @@ -1,3 +1,6 @@ +#ifndef _LINUX_FS_PIN_H +#define _LINUX_FS_PIN_H + #include <linux/wait.h> struct fs_pin { @@ -16,9 +19,12 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *)) INIT_HLIST_NODE(&p->s_list); INIT_HLIST_NODE(&p->m_list); p->kill = kill; + p->done = 0; } void pin_remove(struct fs_pin *); void pin_insert_group(struct fs_pin *, struct vfsmount *, struct hlist_head *); void pin_insert(struct fs_pin *, struct vfsmount *); void pin_kill(struct fs_pin *); + +#endif
Without initialized, done in fs_pin at stack space may contains strange value. v8, same as v3 Adds macro for header file Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> --- include/linux/fs_pin.h | 6 ++++++ 1 file changed, 6 insertions(+)