| Message ID | 20200925161447.1486883-3-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] blk-cgroup: stop abusing get_gendisk | expand |
Hi! > get_gendisk grabs a reference on the disk and file operation, so this > code will leak both of them while having absolutely no use for the > gendisk itself. > > This effectively reverts commit 2df83fa4bce421f > ("PM / Hibernate: Use get_gendisk to verify partition if resume_file is integer format") > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > kernel/power/hibernate.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c > index e7aa57fb2fdc33..7d0b99d2e69631 100644 > --- a/kernel/power/hibernate.c > +++ b/kernel/power/hibernate.c > @@ -948,17 +948,6 @@ static int software_resume(void) > > /* Check if the device is there */ > swsusp_resume_device = name_to_dev_t(resume_file); > - > - /* > - * name_to_dev_t is ineffective to verify parition if resume_file is in > - * integer format. (e.g. major:minor) > - */ > - if (isdigit(resume_file[0]) && resume_wait) { > - int partno; > - while (!get_gendisk(swsusp_resume_device, &partno)) > - msleep(10); > - } I believe point of this code was to wait for resume device to appear -- see the resume_wait condition. It should not be simply removed. Best regards, Pavel
On Fri, Sep 25, 2020 at 08:38:28PM +0200, Pavel Machek wrote: > > - * name_to_dev_t is ineffective to verify parition if resume_file is in > > - * integer format. (e.g. major:minor) > > - */ > > - if (isdigit(resume_file[0]) && resume_wait) { > > - int partno; > > - while (!get_gendisk(swsusp_resume_device, &partno)) > > - msleep(10); > > - } > > I believe point of this code was to wait for resume device to appear > -- see the resume_wait condition. It should not be simply removed. But get_gendisk has absolutely no relation to a device appearing. So whatever this code tried to do doesn't make any sense.
On Fri, Sep 25, 2020 at 6:15 PM Christoph Hellwig <hch@lst.de> wrote: > > get_gendisk grabs a reference on the disk and file operation, so this > code will leak both of them while having absolutely no use for the > gendisk itself. > > This effectively reverts commit 2df83fa4bce421f > ("PM / Hibernate: Use get_gendisk to verify partition if resume_file is integer format") > > Signed-off-by: Christoph Hellwig <hch@lst.de> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > kernel/power/hibernate.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c > index e7aa57fb2fdc33..7d0b99d2e69631 100644 > --- a/kernel/power/hibernate.c > +++ b/kernel/power/hibernate.c > @@ -948,17 +948,6 @@ static int software_resume(void) > > /* Check if the device is there */ > swsusp_resume_device = name_to_dev_t(resume_file); > - > - /* > - * name_to_dev_t is ineffective to verify parition if resume_file is in > - * integer format. (e.g. major:minor) > - */ > - if (isdigit(resume_file[0]) && resume_wait) { > - int partno; > - while (!get_gendisk(swsusp_resume_device, &partno)) > - msleep(10); > - } > - > if (!swsusp_resume_device) { > /* > * Some device discovery might still be in progress; we need > -- > 2.28.0 >
On Wed, Sep 30, 2020 at 05:45:27PM +0200, Rafael J. Wysocki wrote: > On Fri, Sep 25, 2020 at 6:15 PM Christoph Hellwig <hch@lst.de> wrote: > > > > get_gendisk grabs a reference on the disk and file operation, so this > > code will leak both of them while having absolutely no use for the > > gendisk itself. > > > > This effectively reverts commit 2df83fa4bce421f > > ("PM / Hibernate: Use get_gendisk to verify partition if resume_file is integer format") > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Can you pick it up through the PM tree? The big rework in this area I have planned won't land before 5.11 anyway.
On Fri, Oct 2, 2020 at 8:50 AM Christoph Hellwig <hch@lst.de> wrote: > > On Wed, Sep 30, 2020 at 05:45:27PM +0200, Rafael J. Wysocki wrote: > > On Fri, Sep 25, 2020 at 6:15 PM Christoph Hellwig <hch@lst.de> wrote: > > > > > > get_gendisk grabs a reference on the disk and file operation, so this > > > code will leak both of them while having absolutely no use for the > > > gendisk itself. > > > > > > This effectively reverts commit 2df83fa4bce421f > > > ("PM / Hibernate: Use get_gendisk to verify partition if resume_file is integer format") > > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Can you pick it up through the PM tree? The big rework in this area > I have planned won't land before 5.11 anyway. Will do, thanks!
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index e7aa57fb2fdc33..7d0b99d2e69631 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -948,17 +948,6 @@ static int software_resume(void) /* Check if the device is there */ swsusp_resume_device = name_to_dev_t(resume_file); - - /* - * name_to_dev_t is ineffective to verify parition if resume_file is in - * integer format. (e.g. major:minor) - */ - if (isdigit(resume_file[0]) && resume_wait) { - int partno; - while (!get_gendisk(swsusp_resume_device, &partno)) - msleep(10); - } - if (!swsusp_resume_device) { /* * Some device discovery might still be in progress; we need
get_gendisk grabs a reference on the disk and file operation, so this code will leak both of them while having absolutely no use for the gendisk itself. This effectively reverts commit 2df83fa4bce421f ("PM / Hibernate: Use get_gendisk to verify partition if resume_file is integer format") Signed-off-by: Christoph Hellwig <hch@lst.de> --- kernel/power/hibernate.c | 11 ----------- 1 file changed, 11 deletions(-)