* [PATCH 0/4] introduce new put_getdisk() call
@ 2016-02-01 14:51 ` Roman Pen
0 siblings, 0 replies; 10+ messages in thread
From: Roman Pen @ 2016-02-01 14:51 UTC (permalink / raw)
Cc: Roman Pen, Martin K. Petersen, Rafael J. Wysocki, Alexander Viro,
Dan Williams, Gi-Oh Kim, Jens Axboe, Len Brown, Ming Lei,
Pavel Machek, Sagi Grimberg, Tejun Heo, Vishal Verma,
linux-block, linux-fsdevel, linux-kernel, linux-pm
Hello.
In this patchset in the first patch I fixed module reference leak inside
blk-cgroup.c. In other patches I switched to a new put_getdisk() call,
which should be used if the disk was received by get_disk() or get_gendisk()
functions, which internally increase module reference.
The idea is to avoid confusion in the future and to have symmetric calls:
alloc_disk() -> put_disk() [as it is done in all the block drivers]
and
get_gendisk() -> put_gendisk() [if you need to find a disk by minor,major]
The second sequence internally increases disk owner module reference on
get and decreases it on put.
Roman Pen (4):
block: fix module reference leak on put_disk() call for cgroups
throttle
block: introduce new call put_gendisk() in genhd.c
block,fs: switch to a new put_gendisk() call
hibernate: fix disk and module leak on successfull resume
block/blk-cgroup.c | 6 ++---
block/genhd.c | 59 +++++++++++++++++++++++++++++++++++++++++++++---
fs/block_dev.c | 24 ++++++--------------
include/linux/genhd.h | 1 +
kernel/power/hibernate.c | 5 +++-
5 files changed, 71 insertions(+), 24 deletions(-)
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Gi-Oh Kim <gi-oh.kim@profitbricks.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Len Brown <len.brown@intel.com>
Cc: Ming Lei <tom.leiming@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: linux-block@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pm@vger.kernel.org
--
2.6.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/4] block: fix module reference leak on put_disk() call for cgroups throttle
2016-02-01 14:51 ` Roman Pen
(?)
@ 2016-02-01 14:51 ` Roman Pen
2016-02-03 10:40 ` Gi-Oh Kim
-1 siblings, 1 reply; 10+ messages in thread
From: Roman Pen @ 2016-02-01 14:51 UTC (permalink / raw)
Cc: Roman Pen, Gi-Oh Kim, Tejun Heo, Jens Axboe, linux-block, linux-kernel
get_disk(),get_gendisk() calls have non explicit side effect: they
increase the reference on the disk owner module.
The following is the correct sequence how to get a disk reference and
to put it:
disk = get_gendisk(...);
/* use disk */
owner = disk->fops->owner;
put_disk(disk);
module_put(owner);
fs/block_dev.c is aware of this required module_put() call, but f.e.
blkg_conf_finish(), which is located in block/blk-cgroup.c, does not put
a module reference. To see a leakage in action cgroups throttle config
can be used. In the following script I'm removing throttle for /dev/ram0
(actually this is NOP, because throttle was never set for this device):
# lsmod | grep brd
brd 5175 0
# i=100; while [ $i -gt 0 ]; do echo "1:0 0" > \
/sys/fs/cgroup/blkio/blkio.throttle.read_bps_device; i=$(($i - 1)); \
done
# lsmod | grep brd
brd 5175 100
Now brd module has 100 references.
The issue is fixed by calling module_put() just right away put_disk().
Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
Cc: Gi-Oh Kim <gi-oh.kim@profitbricks.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
block/blk-cgroup.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 5a37188..66e6f1a 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -788,6 +788,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
{
struct gendisk *disk;
struct blkcg_gq *blkg;
+ struct module *owner;
unsigned int major, minor;
int key_len, part, ret;
char *body;
@@ -804,7 +805,9 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
if (!disk)
return -ENODEV;
if (part) {
+ owner = disk->fops->owner;
put_disk(disk);
+ module_put(owner);
return -ENODEV;
}
@@ -820,7 +823,9 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
ret = PTR_ERR(blkg);
rcu_read_unlock();
spin_unlock_irq(disk->queue->queue_lock);
+ owner = disk->fops->owner;
put_disk(disk);
+ module_put(owner);
/*
* If queue was bypassing, we should retry. Do so after a
* short msleep(). It isn't strictly necessary but queue
@@ -851,9 +856,13 @@ EXPORT_SYMBOL_GPL(blkg_conf_prep);
void blkg_conf_finish(struct blkg_conf_ctx *ctx)
__releases(ctx->disk->queue->queue_lock) __releases(rcu)
{
+ struct module *owner;
+
spin_unlock_irq(ctx->disk->queue->queue_lock);
rcu_read_unlock();
+ owner = ctx->disk->fops->owner;
put_disk(ctx->disk);
+ module_put(owner);
}
EXPORT_SYMBOL_GPL(blkg_conf_finish);
--
2.6.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/4] block: fix module reference leak on put_disk() call for cgroups throttle
2016-02-01 14:51 ` [PATCH 1/4] block: fix module reference leak on put_disk() call for cgroups throttle Roman Pen
@ 2016-02-03 10:40 ` Gi-Oh Kim
0 siblings, 0 replies; 10+ messages in thread
From: Gi-Oh Kim @ 2016-02-03 10:40 UTC (permalink / raw)
To: Roman Pen; +Cc: Tejun Heo, Jens Axboe, linux-block, linux-kernel
On 01.02.2016 15:51, Roman Pen wrote:
> get_disk(),get_gendisk() calls have non explicit side effect: they
> increase the reference on the disk owner module.
>
> The following is the correct sequence how to get a disk reference and
> to put it:
>
> disk = get_gendisk(...);
>
> /* use disk */
>
> owner = disk->fops->owner;
> put_disk(disk);
> module_put(owner);
>
> fs/block_dev.c is aware of this required module_put() call, but f.e.
> blkg_conf_finish(), which is located in block/blk-cgroup.c, does not put
> a module reference. To see a leakage in action cgroups throttle config
> can be used. In the following script I'm removing throttle for /dev/ram0
> (actually this is NOP, because throttle was never set for this device):
>
> # lsmod | grep brd
> brd 5175 0
> # i=100; while [ $i -gt 0 ]; do echo "1:0 0" > \
> /sys/fs/cgroup/blkio/blkio.throttle.read_bps_device; i=$(($i - 1)); \
> done
> # lsmod | grep brd
> brd 5175 100
>
> Now brd module has 100 references.
I hope this patch would be merged into stable versions.
I've tested the patch with v3.12.45 on my server.
Before apply the patch, reference count was leaked.
# lsmod | grep loop
loop 16091 0
# echo "7:0 1048576" > /sys/fs/cgroup/blkio/blkio.throttle.read_bps_device
# lsmod | grep loop
loop 16091 1
# echo "7:0 1048576" > /sys/fs/cgroup/blkio/blkio.throttle.read_bps_device
# lsmod | grep loop
loop 16091 2
# echo "7:0 1048576" > /sys/fs/cgroup/blkio/blkio.throttle.read_bps_device
# lsmod | grep loop
loop 16091 3
# echo "7:0 1048576" > /sys/fs/cgroup/blkio/blkio.throttle.read_bps_device
# lsmod | grep loop
loop 16091 4
After apply the patch, reference count was still:
# lsmod | grep loop
loop 19171 0
# echo "7:0 1048576" > /sys/fs/cgroup/blkio/blkio.throttle.read_bps_device
# lsmod | grep loop
loop 19171 0
# echo "7:0 1048576" > /sys/fs/cgroup/blkio/blkio.throttle.read_bps_device
# lsmod | grep loop
loop 19171 0
# echo "7:0 1048576" > /sys/fs/cgroup/blkio/blkio.throttle.read_bps_device
# lsmod | grep loop
loop 19171 0
--
Best regards,
Gi-Oh Kim
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] block: introduce new call put_gendisk() in genhd.c
2016-02-01 14:51 ` Roman Pen
(?)
(?)
@ 2016-02-01 14:51 ` Roman Pen
-1 siblings, 0 replies; 10+ messages in thread
From: Roman Pen @ 2016-02-01 14:51 UTC (permalink / raw)
Cc: Roman Pen, Gi-Oh Kim, Jens Axboe, Dan Williams, Sagi Grimberg,
Martin K. Petersen, Ming Lei, Vishal Verma, linux-block,
linux-kernel
The confusion comes from the fact, that if disk was get using
get_disk()
get_gendisk()
calls, you also have to put a disk owner module reference, so the sequence
is the following:
disk = get_gendisk(...);
/* use disk */
owner = disk->fops->owner;
put_disk(disk);
module_put(owner);
The disk, which was allocated using alloc_disk() requires only the opposite
put_disk() call, without puting the module reference.
That's error prone.
Here in this patch new call is introduced put_gendisk(), which is aimed to
put a disk reference and also a disk owner reference.
The expected usage for the block drivers has not been changed and is the
following:
disk = alloc_disk(...);
/* use */
put_disk(disk);
The expected usage for those who did get_gendisk():
disk = get_gendisk();
/* use */
put_gendisk(disk);
That should help to get rid of modules references leak, which happens if
disk was received by get_gendisk() call, but then was put by put_disk(),
without corresponding module_put().
Also function description is updated.
Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
Cc: Gi-Oh Kim <gi-oh.kim@profitbricks.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Ming Lei <tom.leiming@gmail.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: linux-block@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
block/genhd.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++---
include/linux/genhd.h | 1 +
2 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 0f566a5..66c8278 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -722,7 +722,13 @@ static ssize_t disk_badblocks_store(struct device *dev,
* @partno: returned partition index
*
* This function gets the structure containing partitioning
- * information for the given device @devt.
+ * information for the given device @devt and increases the kobj
+ * and disk owner module references.
+ *
+ * RETURNS:
+ * Resulting gendisk on success, NULL in case of failure. After usage
+ * disk must be put with put_gendisk() call, which also puts the module
+ * reference.
*/
struct gendisk *get_gendisk(dev_t devt, int *partno)
{
@@ -1305,6 +1311,17 @@ struct gendisk *alloc_disk(int minors)
}
EXPORT_SYMBOL(alloc_disk);
+/**
+ * alloc_disk_node - allocates disk for specified minors and node id.
+ *
+ * @minors: how many minors are expected for that disk before switching
+ * to extended block range
+ * @node_id: memory node from which to allocate
+ *
+ * RETURNS:
+ * Resulting disk on success, NULL in case of failure.
+ * alloc_disk() and alloc_disk_node() are paired with put_disk() call.
+ */
struct gendisk *alloc_disk_node(int minors, int node_id)
{
struct gendisk *disk;
@@ -1349,6 +1366,16 @@ struct gendisk *alloc_disk_node(int minors, int node_id)
}
EXPORT_SYMBOL(alloc_disk_node);
+/**
+ * get_disk - increases the kobj and module references.
+ *
+ * @disk: disk for which references should be increased
+ *
+ * RETURNS:
+ * Resulting kobj of the disk on success, NULL in case of failure.
+ * After usage disk must be put with put_gendisk() call, which also
+ * puts the module reference.
+ */
struct kobject *get_disk(struct gendisk *disk)
{
struct module *owner;
@@ -1367,17 +1394,43 @@ struct kobject *get_disk(struct gendisk *disk)
return kobj;
}
-
EXPORT_SYMBOL(get_disk);
+/**
+ * put_disk - puts only kobj reference.
+ *
+ * @disk: disk to put
+ *
+ * This put_disk() is paired with alloc_disk(). Never call this
+ * if you get a disk using get_gendisk() or get_disk() calls.
+ */
void put_disk(struct gendisk *disk)
{
if (disk)
kobject_put(&disk_to_dev(disk)->kobj);
}
-
EXPORT_SYMBOL(put_disk);
+/**
+ * put_gendisk - puts kobj and disk owner module references.
+ *
+ * @disk: disk to put
+ *
+ * This put_gendisk() is paired with get_gendisk() and get_disk() calls.
+ * Never use this call if you just have allocated a disk using alloc_disk(),
+ * use put_disk() instead.
+ */
+void put_gendisk(struct gendisk *disk)
+{
+ if (disk) {
+ struct module *owner = disk->fops->owner;
+
+ put_disk(disk);
+ module_put(owner);
+ }
+}
+EXPORT_SYMBOL(put_gendisk);
+
static void set_disk_ro_uevent(struct gendisk *gd, int ro)
{
char event[] = "DISK_RO=1";
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 9e4e547..f84efbf 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -635,6 +635,7 @@ extern struct gendisk *alloc_disk_node(int minors, int node_id);
extern struct gendisk *alloc_disk(int minors);
extern struct kobject *get_disk(struct gendisk *disk);
extern void put_disk(struct gendisk *disk);
+extern void put_gendisk(struct gendisk *disk);
extern void blk_register_region(dev_t devt, unsigned long range,
struct module *module,
struct kobject *(*probe)(dev_t, int *, void *),
--
2.6.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/4] block,fs: switch to a new put_gendisk() call
2016-02-01 14:51 ` Roman Pen
` (2 preceding siblings ...)
(?)
@ 2016-02-01 14:51 ` Roman Pen
-1 siblings, 0 replies; 10+ messages in thread
From: Roman Pen @ 2016-02-01 14:51 UTC (permalink / raw)
Cc: Roman Pen, Gi-Oh Kim, Tejun Heo, Jens Axboe, Alexander Viro,
linux-block, linux-kernel, linux-fsdevel
In previous patch a new put_gendisk() call was introduced, which puts the
disk and the disk owner module references.
This new call should help not to forget about required module_put() after
receiving a disk pointer using get_disk() or get_gendisk() calls.
In this patch disk_put(),module_put() sequences are replaced with a single
put_gendisk() call.
Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
Cc: Gi-Oh Kim <gi-oh.kim@profitbricks.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-block@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
---
block/blk-cgroup.c | 15 +++------------
fs/block_dev.c | 24 +++++++-----------------
2 files changed, 10 insertions(+), 29 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 66e6f1a..2ed9636 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -788,7 +788,6 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
{
struct gendisk *disk;
struct blkcg_gq *blkg;
- struct module *owner;
unsigned int major, minor;
int key_len, part, ret;
char *body;
@@ -805,9 +804,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
if (!disk)
return -ENODEV;
if (part) {
- owner = disk->fops->owner;
- put_disk(disk);
- module_put(owner);
+ put_gendisk(disk);
return -ENODEV;
}
@@ -823,9 +820,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
ret = PTR_ERR(blkg);
rcu_read_unlock();
spin_unlock_irq(disk->queue->queue_lock);
- owner = disk->fops->owner;
- put_disk(disk);
- module_put(owner);
+ put_gendisk(disk);
/*
* If queue was bypassing, we should retry. Do so after a
* short msleep(). It isn't strictly necessary but queue
@@ -856,13 +851,9 @@ EXPORT_SYMBOL_GPL(blkg_conf_prep);
void blkg_conf_finish(struct blkg_conf_ctx *ctx)
__releases(ctx->disk->queue->queue_lock) __releases(rcu)
{
- struct module *owner;
-
spin_unlock_irq(ctx->disk->queue->queue_lock);
rcu_read_unlock();
- owner = ctx->disk->fops->owner;
- put_disk(ctx->disk);
- module_put(owner);
+ put_gendisk(ctx->disk);
}
EXPORT_SYMBOL_GPL(blkg_conf_finish);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 7b9cd49..dc2ea76 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -15,7 +15,6 @@
#include <linux/highmem.h>
#include <linux/blkdev.h>
#include <linux/backing-dev.h>
-#include <linux/module.h>
#include <linux/blkpg.h>
#include <linux/magic.h>
#include <linux/buffer_head.h>
@@ -870,8 +869,7 @@ static struct block_device *bd_start_claiming(struct block_device *bdev,
else
whole = bdgrab(bdev);
- module_put(disk->fops->owner);
- put_disk(disk);
+ put_gendisk(disk);
if (!whole)
return ERR_PTR(-ENOMEM);
@@ -1167,7 +1165,6 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part);
static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
{
struct gendisk *disk;
- struct module *owner;
int ret;
int partno;
int perm = 0;
@@ -1193,7 +1190,6 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
disk = get_gendisk(bdev->bd_dev, &partno);
if (!disk)
goto out;
- owner = disk->fops->owner;
disk_block_events(disk);
mutex_lock_nested(&bdev->bd_mutex, for_part);
@@ -1222,8 +1218,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
bdev->bd_queue = NULL;
mutex_unlock(&bdev->bd_mutex);
disk_unblock_events(disk);
- put_disk(disk);
- module_put(owner);
+ put_gendisk(disk);
goto restart;
}
}
@@ -1285,9 +1280,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
if (ret)
goto out_unlock_bdev;
}
- /* only one opener holds refs to the module and disk */
- put_disk(disk);
- module_put(owner);
+ /* only one opener holds refs to the disk */
+ put_gendisk(disk);
}
bdev->bd_openers++;
if (for_part)
@@ -1307,8 +1301,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
out_unlock_bdev:
mutex_unlock(&bdev->bd_mutex);
disk_unblock_events(disk);
- put_disk(disk);
- module_put(owner);
+ put_gendisk(disk);
out:
bdput(bdev);
@@ -1525,7 +1518,7 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
/*
* Detaching bdev inode from its wb in __destroy_inode()
* is too late: the queue which embeds its bdi (along with
- * root wb) can be gone as soon as we put_disk() below.
+ * root wb) can be gone as soon as we put_gendisk() below.
*/
inode_detach_wb(bdev->bd_inode);
}
@@ -1534,8 +1527,6 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
disk->fops->release(disk, mode);
}
if (!bdev->bd_openers) {
- struct module *owner = disk->fops->owner;
-
disk_put_part(bdev->bd_part);
bdev->bd_part = NULL;
bdev->bd_disk = NULL;
@@ -1543,8 +1534,7 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
victim = bdev->bd_contains;
bdev->bd_contains = NULL;
- put_disk(disk);
- module_put(owner);
+ put_gendisk(disk);
}
mutex_unlock(&bdev->bd_mutex);
bdput(bdev);
--
2.6.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] hibernate: fix disk and module leak on successfull resume
2016-02-01 14:51 ` Roman Pen
@ 2016-02-01 14:51 ` Roman Pen
-1 siblings, 0 replies; 10+ messages in thread
From: Roman Pen @ 2016-02-01 14:51 UTC (permalink / raw)
Cc: Roman Pen, Gi-Oh Kim, Rafael J. Wysocki, Len Brown, Pavel Machek,
linux-pm, linux-kernel
Do not forget to put the disk back.
Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
Cc: Gi-Oh Kim <gi-oh.kim@profitbricks.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <len.brown@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
kernel/power/hibernate.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index b7342a2..1f53dc2 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -785,8 +785,11 @@ static int software_resume(void)
*/
if (isdigit(resume_file[0]) && resume_wait) {
int partno;
- while (!get_gendisk(swsusp_resume_device, &partno))
+ struct gendisk *disk;
+
+ while (!(disk = get_gendisk(swsusp_resume_device, &partno)))
msleep(10);
+ put_gendisk(disk);
}
if (!swsusp_resume_device) {
--
2.6.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] hibernate: fix disk and module leak on successfull resume
@ 2016-02-01 14:51 ` Roman Pen
0 siblings, 0 replies; 10+ messages in thread
From: Roman Pen @ 2016-02-01 14:51 UTC (permalink / raw)
Cc: Roman Pen, Gi-Oh Kim, Rafael J. Wysocki, Len Brown, Pavel Machek,
linux-pm, linux-kernel
Do not forget to put the disk back.
Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
Cc: Gi-Oh Kim <gi-oh.kim@profitbricks.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <len.brown@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
kernel/power/hibernate.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index b7342a2..1f53dc2 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -785,8 +785,11 @@ static int software_resume(void)
*/
if (isdigit(resume_file[0]) && resume_wait) {
int partno;
- while (!get_gendisk(swsusp_resume_device, &partno))
+ struct gendisk *disk;
+
+ while (!(disk = get_gendisk(swsusp_resume_device, &partno)))
msleep(10);
+ put_gendisk(disk);
}
if (!swsusp_resume_device) {
--
2.6.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/4] hibernate: fix disk and module leak on successfull resume
2016-02-01 14:51 ` Roman Pen
(?)
@ 2016-02-03 1:37 ` Rafael J. Wysocki
-1 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-02-03 1:37 UTC (permalink / raw)
To: Roman Pen; +Cc: Gi-Oh Kim, Len Brown, Pavel Machek, linux-pm, linux-kernel
On Monday, February 01, 2016 03:51:55 PM Roman Pen wrote:
> Do not forget to put the disk back.
>
> Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
> Cc: Gi-Oh Kim <gi-oh.kim@profitbricks.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> kernel/power/hibernate.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index b7342a2..1f53dc2 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -785,8 +785,11 @@ static int software_resume(void)
> */
> if (isdigit(resume_file[0]) && resume_wait) {
> int partno;
> - while (!get_gendisk(swsusp_resume_device, &partno))
> + struct gendisk *disk;
> +
> + while (!(disk = get_gendisk(swsusp_resume_device, &partno)))
> msleep(10);
> + put_gendisk(disk);
> }
>
> if (!swsusp_resume_device) {
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] introduce new put_getdisk() call
2016-02-01 14:51 ` Roman Pen
` (4 preceding siblings ...)
(?)
@ 2016-02-01 17:16 ` Tejun Heo
-1 siblings, 0 replies; 10+ messages in thread
From: Tejun Heo @ 2016-02-01 17:16 UTC (permalink / raw)
To: Roman Pen
Cc: Martin K. Petersen, Rafael J. Wysocki, Alexander Viro,
Dan Williams, Gi-Oh Kim, Jens Axboe, Len Brown, Ming Lei,
Pavel Machek, Sagi Grimberg, Vishal Verma, linux-block,
linux-fsdevel, linux-kernel, linux-pm
On Mon, Feb 01, 2016 at 03:51:51PM +0100, Roman Pen wrote:
> Hello.
>
> In this patchset in the first patch I fixed module reference leak inside
> blk-cgroup.c. In other patches I switched to a new put_getdisk() call,
> which should be used if the disk was received by get_disk() or get_gendisk()
> functions, which internally increase module reference.
>
> The idea is to avoid confusion in the future and to have symmetric calls:
>
> alloc_disk() -> put_disk() [as it is done in all the block drivers]
>
> and
>
> get_gendisk() -> put_gendisk() [if you need to find a disk by minor,major]
>
> The second sequence internally increases disk owner module reference on
> get and decreases it on put.
For the entire series,
Acked-by: Tejun Heo <tj@kernel.org>
Nice catch, thanks!
--
tejun
^ permalink raw reply [flat|nested] 10+ messages in thread