Heiko Carstens wrote:
> On Fri, Jul 17, 2009 at 02:08:46PM +0100, Alan Jenkins wrote:
>
>> Rafael J. Wysocki wrote:
>>
>>> On Tuesday 14 July 2009, Heiko Carstens wrote:
>>>
>>>
>>>> We've seen this bug [...]
>>>>
>>>> Looks like this was introduced with git commit a1bb7d61 "PM/hibernate: fix "swap
>>>> breaks after hibernation failures"".
>>>> Calling bdget while holding a spinlock doesn't seem to be a good idea...
>>>>
>>>>
>>> Agreed, sorry for missing that.
>>>
>>> Alan, can you please prepare a fix?
>>>
>> Here's a quick & dirty patch [...]
>>
>
> Thanks for the patch. Unfortunately Arnd was unable to reproduce the original
> behaviour. But your patch makes sense anyway.
> I also tested it and nothing broke. So should this go upstream?
I do want to fix it, but I think there's a better way.
It doesn't really need the sleeping bdget(). All we want is
atomic_inc(&bdev->bd_inode->i_count). I think we should call it bdcopy().
---------->
>From 643014ec079610a8b01dfd78c6949c1e8727195b Mon Sep 17 00:00:00 2001
From: Alan Jenkins <alan-jenkins@xxxxxxxxxxxxxx>
Date: Tue, 21 Jul 2009 10:17:30 +0100
Subject: [PATCH] PM/hibernate: replace bdget call with bdcopy (simple atomic_inc of i_count)
Create bdcopy(). This function copies an existing reference to a
block_device. It is safe to call from any context.
Hibernation code wishes to copy a reference to the active swap device.
Right now it calls bdget() under a spinlock, but this is wrong because
bdget() can sleep. It doesn't need a full bdget() because we already
hold a reference to active swap devices (and the spinlock protects
against swapoff).
Signed-off-by: Alan Jenkins <alan-jenkins@xxxxxxxxxxxxxx>
CC: linux-fsdevel@xxxxxxxxxxxxxxx
---
fs/block_dev.c | 8 ++++++++
include/linux/fs.h | 1 +
mm/swapfile.c | 4 ++--
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 3a6d4fb..0b04974 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -564,6 +564,14 @@ struct block_device *bdget(dev_t dev)
EXPORT_SYMBOL(bdget);
+struct block_device *bdcopy(struct block_device *bdev)
+{
+ atomic_inc(&bdev->bd_inode->i_count);
+ return bdev;
+}
+
+EXPORT_SYMBOL(bdcopy);
+
long nr_blockdev_pages(void)
{
struct block_device *bdev;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 0872372..eeb1091 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1946,6 +1946,7 @@ extern void putname(const char *name);
extern int register_blkdev(unsigned int, const char *);
extern void unregister_blkdev(unsigned int, const char *);
extern struct block_device *bdget(dev_t);
+extern struct block_device *bdcopy(struct block_device *bdev);
extern void bd_set_size(struct block_device *, loff_t size);
extern void bd_forget(struct inode *inode);
extern void bdput(struct block_device *);
diff --git a/mm/swapfile.c b/mm/swapfile.c
index d1ade1a..272ea8e 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -753,7 +753,7 @@ int swap_type_of(dev_t device, sector_t offset, struct block_device **bdev_p)
if (!bdev) {
if (bdev_p)
- *bdev_p = bdget(sis->bdev->bd_dev);
+ *bdev_p = bdcopy(sis->bdev);
spin_unlock(&swap_lock);
return i;
@@ -765,7 +765,7 @@ int swap_type_of(dev_t device, sector_t offset, struct block_device **bdev_p)
struct swap_extent, list);
if (se->start_block == offset) {
if (bdev_p)
- *bdev_p = bdget(sis->bdev->bd_dev);
+ *bdev_p = bdcopy(sis->bdev);
spin_unlock(&swap_lock);
bdput(bdev);
--
1.6.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
