Messages in this thread | | | Date | Tue, 28 Jun 2022 21:57:26 +0800 (GMT+08:00) | From | duoming@zju ... | Subject | Re: [PATCH v7] devcoredump: change gfp_t parameter of kzalloc to GFP_KERNEL |
| |
Hello,
On Tue, 28 Jun 2022 14:18:29 +0200 greg KH wrote:
> > > > The dev_coredumpv() and dev_coredumpm() could not be used in atomic > > > > context, because they call kvasprintf_const() and kstrdup() with > > > > GFP_KERNEL parameter. The process is shown below: > > > > > > > > dev_coredumpv(.., gfp_t gfp) > > > > dev_coredumpm(.., gfp_t gfp) > > > > kzalloc(.., gfp); > > > > dev_set_name > > > > kobject_set_name_vargs > > > > kvasprintf_const(GFP_KERNEL, ...); //may sleep > > > > kstrdup(s, GFP_KERNEL); //may sleep > > > > > > > > This patch changes the gfp_t parameter of kzalloc() in dev_coredumpm() to > > > > GFP_KERNEL in order to show they could not be used in atomic context. > > > > > > > > What's more, this patch does not remove the gfp_t parameter in > > > > dev_coredumpv() and dev_coredumpm() in order that it will not influence > > > > other new users that are added in other trees. > > > > > > > > Fixes: 833c95456a70 ("device coredump: add new device coredump class") > > > > Signed-off-by: Duoming Zhou <duoming@zju.edu.cn> > > > > --- > > > > Changes in v7: > > > > - change gfp_t parameter of kzalloc in dev_coredumpm() to GFP_KERNEL. > > > > > > > > drivers/base/devcoredump.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c > > > > index f4d794d6bb8..cf60aacf8a8 100644 > > > > --- a/drivers/base/devcoredump.c > > > > +++ b/drivers/base/devcoredump.c > > > > @@ -268,7 +268,7 @@ void dev_coredumpm(struct device *dev, struct module *owner, > > > > if (!try_module_get(owner)) > > > > goto free; > > > > > > > > - devcd = kzalloc(sizeof(*devcd), gfp); > > > > + devcd = kzalloc(sizeof(*devcd), GFP_KERNEL); > > > > > > No, you can't just ignore the flag entirely, that doesn't help anyone > > > out who tries to set it and is totally confused as to why the field is > > > ignored. > > > > > > You need to evolve the function over time to not need the parameter at > > > all, this just papers over the entire issue, which makes the api lie to > > > the caller, not something you ever want to do. > > > > Thank you for your time and reply. > > > > But if there are new devices come into kernel, it may use devcoredump api. > > What is the proper time to remove the gfp_t parameter of dev_coredumpv() > > and dev_coredumpm()? > > Normally you prepare some patches that does the conversion as a patch > series and I queue them up in my tree, and get them merged in -rc1, then > any stragglers are then fixed up in -rc2 along with the final rename of > the old way and then all is good. See lots of examples of changing apis > over time on the mailing lists for how to do this.
Thank you for your time and reply, I understand.
I will resend the patch that removes the gfp_t parameter of dev_coredumpv() and dev_coredumpm() until commit oc3d8785f6c04a ("drm/amdgpu: adding device coredump support") has been merged into mainline. Maybe after v5.19-rc5 or v5.19-rc6.
Best regards, Duoming Zhou | |