[PATCH] dlpar: Fix a missing-check bug in dlpar_parse_cc_property()
In dlpar_parse_cc_property(), 'prop->name' is allocated by kstrdup().
kstrdup() may return NULL, so it should be checked and handle error.
And prop should be freed if 'prop->name' is NULL.
Signed-off-by: Gen Zhang
---
diff --git a/arch/powerpc/platforms/pseries/dlpar.c
b/arch/powerpc/platforms/pseries/dlpar.c
index 1795804..c852024 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -61,6 +61,10 @@ static struct property *dlpar_parse_cc_property(struct
cc_workarea *ccwa)
name = (char *)ccwa + be32_to_cpu(ccwa->name_offset);
prop->name = kstrdup(name, GFP_KERNEL);
+ if (!prop->name) {
+ dlpar_free_cc_property(prop);
+ return NULL;
+ }
prop->length = be32_to_cpu(ccwa->prop_length);
value = (char *)ccwa + be32_to_cpu(ccwa->prop_offset);
---
Re: [PATCH v2] mm: add account_locked_vm utility function
On Fri, 24 May 2019 13:50:45 -0400 Daniel Jordan
wrote:
> locked_vm accounting is done roughly the same way in five places, so
> unify them in a helper. Standardize the debug prints, which vary
> slightly, but include the helper's caller to disambiguate between
> callsites.
>
> Error codes stay the same, so user-visible behavior does too. The one
> exception is that the -EPERM case in tce_account_locked_vm is removed
> because Alexey has never seen it triggered.
>
> ...
>
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1564,6 +1564,25 @@ long get_user_pages_unlocked(unsigned long start,
> unsigned long nr_pages,
> int get_user_pages_fast(unsigned long start, int nr_pages,
> unsigned int gup_flags, struct page **pages);
>
> +int __account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc,
> + struct task_struct *task, bool bypass_rlim);
> +
> +static inline int account_locked_vm(struct mm_struct *mm, unsigned long
> pages,
> + bool inc)
> +{
> + int ret;
> +
> + if (pages == 0 || !mm)
> + return 0;
> +
> + down_write(>mmap_sem);
> + ret = __account_locked_vm(mm, pages, inc, current,
> + capable(CAP_IPC_LOCK));
> + up_write(>mmap_sem);
> +
> + return ret;
> +}
That's quite a mouthful for an inlined function. How about uninlining
the whole thing and fiddling drivers/vfio/vfio_iommu_type1.c to suit.
I wonder why it does down_write_killable and whether it really needs
to...
Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
On Sat, May 25, 2019 at 02:14:07PM -0400, Joel Fernandes wrote: [snip] > > That aside, if we are going to change the name of an API that is > > used 160 places throughout the tree, we would need to have a pretty > > good justification. Without such a justification, it will just look > > like pointless churn to the various developers and maintainers on the > > receiving end of the patches. > > Actually, the API name change is not something I want to do, it is Steven > suggestion. My suggestion is let us just delete _raw_notrace and just use the > _raw API for tracing, since _raw doesn't do any tracing anyway. Steve pointed > that _raw_notrace does sparse checking unlike _raw, but I think that isn't an > issue since _raw doesn't do such checking at the moment anyway.. (if possible > check my cover letter again for details/motivation of this series). Come to think of it, if we/I succeed in adding lockdep checking in _raw, then we can just keep the current APIs and not delete anything. And we can have _raw_notrace skip the lockdep checks. The sparse check question would still be an open one though, since _raw doesn't do sparse checks at the moment unlike _raw_notrace as Steve pointed. Thanks, - Joel
Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
On Sat, May 25, 2019 at 08:50:35AM -0700, Paul E. McKenney wrote: > On Sat, May 25, 2019 at 10:19:54AM -0400, Joel Fernandes wrote: > > On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote: > > > On Sat, 25 May 2019 04:14:44 -0400 > > > Joel Fernandes wrote: > > > > > > > > I guess the difference between the _raw_notrace and just _raw variants > > > > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep > > > > > that check? > > > > > > > > This is true. > > > > > > > > Since the users of _raw_notrace are very few, is it worth keeping this > > > > API > > > > just for sparse checking? The API naming is also confusing. I was > > > > expecting > > > > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I > > > > just > > > > want to nuke _raw_notrace as done in this series and later we can > > > > introduce a > > > > sparse checking version of _raw if need-be. The other option could be to > > > > always do sparse checking for _raw however that used to be the case and > > > > got > > > > changed in > > > > http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html > > > > > > What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ? > > > > That would also mean changing 160 usages of _raw to _raw_nocheck in the > > kernel :-/. > > > > The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just > > call > > rcu_check_sparse directly in the calling code for those and eliminate the > > APIs? > > > > I wonder what Paul thinks about the matter as well. > > My thought is that it is likely that a goodly number of the current uses > of _raw should really be some form of _check, with lockdep expressions > spelled out. Not that working out what exactly those lockdep expressions > should be is necessarily a trivial undertaking. ;-) Yes, currently where I am a bit stuck is the rcu_dereference_raw() cannot possibly know what SRCU domain it is under, so lockdep cannot check if an SRCU lock is held without the user also passing along the SRCU domain. I am trying to change lockdep to see if it can check if *any* srcu domain lock is held (regardless of which one) and complain if none are. This is at least better than no check at all. However, I think it gets tricky for mutexes. If you have something like: mutex_lock(some_mutex); p = rcu_dereference_raw(gp); mutex_unlock(some_mutex); This might be a perfectly valid invocation of _raw, however my checks (patch is still cooking) trigger a lockdep warning becase _raw cannot know that this is Ok. lockdep thinks it is not in a reader section. This then gets into the territory of a new rcu_derference_raw_protected(gp, assert_held(some_mutex)) which sucks because its yet another API. To circumvent this issue, can we just have callers of rcu_dereference_raw ensure that they call rcu_read_lock() if they are protecting dereferences by a mutex? That would make things a lot easier and also may be Ok since rcu_read_lock is quite cheap. > That aside, if we are going to change the name of an API that is > used 160 places throughout the tree, we would need to have a pretty > good justification. Without such a justification, it will just look > like pointless churn to the various developers and maintainers on the > receiving end of the patches. Actually, the API name change is not something I want to do, it is Steven suggestion. My suggestion is let us just delete _raw_notrace and just use the _raw API for tracing, since _raw doesn't do any tracing anyway. Steve pointed that _raw_notrace does sparse checking unlike _raw, but I think that isn't an issue since _raw doesn't do such checking at the moment anyway.. (if possible check my cover letter again for details/motivation of this series). thanks! - Joel > Thanx, Paul > > > thanks, Steven! > > >
Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
On Sat, May 25, 2019 at 10:19:54AM -0400, Joel Fernandes wrote: > On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote: > > On Sat, 25 May 2019 04:14:44 -0400 > > Joel Fernandes wrote: > > > > > > I guess the difference between the _raw_notrace and just _raw variants > > > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep > > > > that check? > > > > > > This is true. > > > > > > Since the users of _raw_notrace are very few, is it worth keeping this API > > > just for sparse checking? The API naming is also confusing. I was > > > expecting > > > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I > > > just > > > want to nuke _raw_notrace as done in this series and later we can > > > introduce a > > > sparse checking version of _raw if need-be. The other option could be to > > > always do sparse checking for _raw however that used to be the case and > > > got > > > changed in > > > http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html > > > > What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ? > > That would also mean changing 160 usages of _raw to _raw_nocheck in the > kernel :-/. > > The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just call > rcu_check_sparse directly in the calling code for those and eliminate the > APIs? > > I wonder what Paul thinks about the matter as well. My thought is that it is likely that a goodly number of the current uses of _raw should really be some form of _check, with lockdep expressions spelled out. Not that working out what exactly those lockdep expressions should be is necessarily a trivial undertaking. ;-) That aside, if we are going to change the name of an API that is used 160 places throughout the tree, we would need to have a pretty good justification. Without such a justification, it will just look like pointless churn to the various developers and maintainers on the receiving end of the patches. Thanx, Paul > thanks, Steven! >
Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote: > On Sat, 25 May 2019 04:14:44 -0400 > Joel Fernandes wrote: > > > > I guess the difference between the _raw_notrace and just _raw variants > > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep > > > that check? > > > > This is true. > > > > Since the users of _raw_notrace are very few, is it worth keeping this API > > just for sparse checking? The API naming is also confusing. I was expecting > > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just > > want to nuke _raw_notrace as done in this series and later we can introduce > > a > > sparse checking version of _raw if need-be. The other option could be to > > always do sparse checking for _raw however that used to be the case and got > > changed in > > http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html > > What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ? That would also mean changing 160 usages of _raw to _raw_nocheck in the kernel :-/. The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just call rcu_check_sparse directly in the calling code for those and eliminate the APIs? I wonder what Paul thinks about the matter as well. thanks, Steven!
Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
On Sat, 25 May 2019 04:14:44 -0400 Joel Fernandes wrote: > > I guess the difference between the _raw_notrace and just _raw variants > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep > > that check? > > This is true. > > Since the users of _raw_notrace are very few, is it worth keeping this API > just for sparse checking? The API naming is also confusing. I was expecting > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just > want to nuke _raw_notrace as done in this series and later we can introduce a > sparse checking version of _raw if need-be. The other option could be to > always do sparse checking for _raw however that used to be the case and got > changed in > http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ? -- Steve
Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
On Fri, May 24, 2019 at 11:24:58PM -0400, Steven Rostedt wrote: > On Fri, 24 May 2019 19:49:28 -0400 > "Joel Fernandes (Google)" wrote: > > > The series removes users of the following APIs, and the APIs themselves, > > since > > the regular non - _notrace variants don't do any tracing anyway. > > * hlist_for_each_entry_rcu_notrace > > * rcu_dereference_raw_notrace > > > > I guess the difference between the _raw_notrace and just _raw variants > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep > that check? This is true. Since the users of _raw_notrace are very few, is it worth keeping this API just for sparse checking? The API naming is also confusing. I was expecting _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just want to nuke _raw_notrace as done in this series and later we can introduce a sparse checking version of _raw if need-be. The other option could be to always do sparse checking for _raw however that used to be the case and got changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html thanks a lot, - Joel > > -- Steve
