Re: [PATCH 3/8] mm/swap: prevent concurrent swapon on the same S_ISBLK blockdev
On Fri, 18 Apr 2014, Weijie Yang wrote: > On Tue, Feb 4, 2014 at 12:20 PM, Hugh Dickins wrote: > >> > >> Truly, I am fed up with silly swapon/swapoff races. How often does > >> anyone call these things? Let's slap a huge lock around the whole > >> thing and be done with it? > > > > That answer makes me sad: we can't be bothered to get it right, > > even when Weijie goes to the trouble of presenting a series to do so. > > But I sure don't deserve a vote until I've actually looked through it. > > Hi, > > This is a ping email. Could I get some options about these patch series? Sorry, this is no more than a pong in return: I've not lost or forgotten these, I shall get to them, but priorities intervene. Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/8] mm/swap: prevent concurrent swapon on the same S_ISBLK blockdev
On Fri, 18 Apr 2014, Weijie Yang wrote: On Tue, Feb 4, 2014 at 12:20 PM, Hugh Dickins hu...@google.com wrote: Truly, I am fed up with silly swapon/swapoff races. How often does anyone call these things? Let's slap a huge lock around the whole thing and be done with it? That answer makes me sad: we can't be bothered to get it right, even when Weijie goes to the trouble of presenting a series to do so. But I sure don't deserve a vote until I've actually looked through it. Hi, This is a ping email. Could I get some options about these patch series? Sorry, this is no more than a pong in return: I've not lost or forgotten these, I shall get to them, but priorities intervene. Hugh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/8] mm/swap: prevent concurrent swapon on the same S_ISBLK blockdev
On Tue, Feb 4, 2014 at 12:20 PM, Hugh Dickins wrote: > On Mon, 3 Feb 2014, Andrew Morton wrote: >> On Mon, 27 Jan 2014 18:03:04 +0800 Weijie Yang >> wrote: >> >> > When swapon the same S_ISBLK blockdev concurrent, the allocated two >> > swap_info could hold the same block_device, because claim_swapfile() >> > allow the same holder(here, it is sys_swapon function). >> > >> > To prevent this situation, This patch adds swap_lock protect to ensure >> > we can find this situation and return -EBUSY for one swapon call. >> > >> > As for S_ISREG swapfile, claim_swapfile() already prevent this scenario >> > by holding inode->i_mutex. >> > >> > This patch is just for a rare scenario, aim to correct of code. >> > >> >> hm, OK. Would it be saner to pass a unique `holder' to >> claim_swapfile()? Say, `p'? >> >> Truly, I am fed up with silly swapon/swapoff races. How often does >> anyone call these things? Let's slap a huge lock around the whole >> thing and be done with it? > > That answer makes me sad: we can't be bothered to get it right, > even when Weijie goes to the trouble of presenting a series to do so. > But I sure don't deserve a vote until I've actually looked through it. > Hi, This is a ping email. Could I get some options about these patch series? Thanks. > Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/8] mm/swap: prevent concurrent swapon on the same S_ISBLK blockdev
On Tue, Feb 4, 2014 at 12:20 PM, Hugh Dickins hu...@google.com wrote: On Mon, 3 Feb 2014, Andrew Morton wrote: On Mon, 27 Jan 2014 18:03:04 +0800 Weijie Yang weijie.y...@samsung.com wrote: When swapon the same S_ISBLK blockdev concurrent, the allocated two swap_info could hold the same block_device, because claim_swapfile() allow the same holder(here, it is sys_swapon function). To prevent this situation, This patch adds swap_lock protect to ensure we can find this situation and return -EBUSY for one swapon call. As for S_ISREG swapfile, claim_swapfile() already prevent this scenario by holding inode-i_mutex. This patch is just for a rare scenario, aim to correct of code. hm, OK. Would it be saner to pass a unique `holder' to claim_swapfile()? Say, `p'? Truly, I am fed up with silly swapon/swapoff races. How often does anyone call these things? Let's slap a huge lock around the whole thing and be done with it? That answer makes me sad: we can't be bothered to get it right, even when Weijie goes to the trouble of presenting a series to do so. But I sure don't deserve a vote until I've actually looked through it. Hi, This is a ping email. Could I get some options about these patch series? Thanks. Hugh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/8] mm/swap: prevent concurrent swapon on the same S_ISBLK blockdev
On Mon, 3 Feb 2014, Andrew Morton wrote: > On Mon, 27 Jan 2014 18:03:04 +0800 Weijie Yang > wrote: > > > When swapon the same S_ISBLK blockdev concurrent, the allocated two > > swap_info could hold the same block_device, because claim_swapfile() > > allow the same holder(here, it is sys_swapon function). > > > > To prevent this situation, This patch adds swap_lock protect to ensure > > we can find this situation and return -EBUSY for one swapon call. > > > > As for S_ISREG swapfile, claim_swapfile() already prevent this scenario > > by holding inode->i_mutex. > > > > This patch is just for a rare scenario, aim to correct of code. > > > > hm, OK. Would it be saner to pass a unique `holder' to > claim_swapfile()? Say, `p'? > > Truly, I am fed up with silly swapon/swapoff races. How often does > anyone call these things? Let's slap a huge lock around the whole > thing and be done with it? That answer makes me sad: we can't be bothered to get it right, even when Weijie goes to the trouble of presenting a series to do so. But I sure don't deserve a vote until I've actually looked through it. Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/8] mm/swap: prevent concurrent swapon on the same S_ISBLK blockdev
On Mon, 27 Jan 2014 18:03:04 +0800 Weijie Yang wrote: > When swapon the same S_ISBLK blockdev concurrent, the allocated two > swap_info could hold the same block_device, because claim_swapfile() > allow the same holder(here, it is sys_swapon function). > > To prevent this situation, This patch adds swap_lock protect to ensure > we can find this situation and return -EBUSY for one swapon call. > > As for S_ISREG swapfile, claim_swapfile() already prevent this scenario > by holding inode->i_mutex. > > This patch is just for a rare scenario, aim to correct of code. > hm, OK. Would it be saner to pass a unique `holder' to claim_swapfile()? Say, `p'? Truly, I am fed up with silly swapon/swapoff races. How often does anyone call these things? Let's slap a huge lock around the whole thing and be done with it? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/8] mm/swap: prevent concurrent swapon on the same S_ISBLK blockdev
On Mon, 27 Jan 2014 18:03:04 +0800 Weijie Yang weijie.y...@samsung.com wrote: When swapon the same S_ISBLK blockdev concurrent, the allocated two swap_info could hold the same block_device, because claim_swapfile() allow the same holder(here, it is sys_swapon function). To prevent this situation, This patch adds swap_lock protect to ensure we can find this situation and return -EBUSY for one swapon call. As for S_ISREG swapfile, claim_swapfile() already prevent this scenario by holding inode-i_mutex. This patch is just for a rare scenario, aim to correct of code. hm, OK. Would it be saner to pass a unique `holder' to claim_swapfile()? Say, `p'? Truly, I am fed up with silly swapon/swapoff races. How often does anyone call these things? Let's slap a huge lock around the whole thing and be done with it? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/8] mm/swap: prevent concurrent swapon on the same S_ISBLK blockdev
On Mon, 3 Feb 2014, Andrew Morton wrote: On Mon, 27 Jan 2014 18:03:04 +0800 Weijie Yang weijie.y...@samsung.com wrote: When swapon the same S_ISBLK blockdev concurrent, the allocated two swap_info could hold the same block_device, because claim_swapfile() allow the same holder(here, it is sys_swapon function). To prevent this situation, This patch adds swap_lock protect to ensure we can find this situation and return -EBUSY for one swapon call. As for S_ISREG swapfile, claim_swapfile() already prevent this scenario by holding inode-i_mutex. This patch is just for a rare scenario, aim to correct of code. hm, OK. Would it be saner to pass a unique `holder' to claim_swapfile()? Say, `p'? Truly, I am fed up with silly swapon/swapoff races. How often does anyone call these things? Let's slap a huge lock around the whole thing and be done with it? That answer makes me sad: we can't be bothered to get it right, even when Weijie goes to the trouble of presenting a series to do so. But I sure don't deserve a vote until I've actually looked through it. Hugh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/8] mm/swap: prevent concurrent swapon on the same S_ISBLK blockdev
When swapon the same S_ISBLK blockdev concurrent, the allocated two
swap_info could hold the same block_device, because claim_swapfile()
allow the same holder(here, it is sys_swapon function).
To prevent this situation, This patch adds swap_lock protect to ensure
we can find this situation and return -EBUSY for one swapon call.
As for S_ISREG swapfile, claim_swapfile() already prevent this scenario
by holding inode->i_mutex.
This patch is just for a rare scenario, aim to correct of code.
Signed-off-by: Weijie Yang
---
mm/swapfile.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 4d24158..413c213 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -2459,9 +2459,10 @@ SYSCALL_DEFINE2(swapon, const char __user *,
specialfile, int, swap_flags)
goto bad_swap;
}
+ /* prevent concurrent swapon on the same S_ISBLK blockdev */
+ spin_lock(_lock);
p->swap_file = swap_file;
mapping = swap_file->f_mapping;
-
for (i = 0; i < nr_swapfiles; i++) {
struct swap_info_struct *q = swap_info[i];
@@ -2472,6 +2473,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile,
int, swap_flags)
goto bad_swap;
}
}
+ spin_unlock(_lock);
inode = mapping->host;
/* If S_ISREG(inode->i_mode) will do mutex_lock(>i_mutex); */
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/8] mm/swap: prevent concurrent swapon on the same S_ISBLK blockdev
When swapon the same S_ISBLK blockdev concurrent, the allocated two
swap_info could hold the same block_device, because claim_swapfile()
allow the same holder(here, it is sys_swapon function).
To prevent this situation, This patch adds swap_lock protect to ensure
we can find this situation and return -EBUSY for one swapon call.
As for S_ISREG swapfile, claim_swapfile() already prevent this scenario
by holding inode-i_mutex.
This patch is just for a rare scenario, aim to correct of code.
Signed-off-by: Weijie Yang weijie.y...@samsung.com
---
mm/swapfile.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 4d24158..413c213 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -2459,9 +2459,10 @@ SYSCALL_DEFINE2(swapon, const char __user *,
specialfile, int, swap_flags)
goto bad_swap;
}
+ /* prevent concurrent swapon on the same S_ISBLK blockdev */
+ spin_lock(swap_lock);
p-swap_file = swap_file;
mapping = swap_file-f_mapping;
-
for (i = 0; i nr_swapfiles; i++) {
struct swap_info_struct *q = swap_info[i];
@@ -2472,6 +2473,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile,
int, swap_flags)
goto bad_swap;
}
}
+ spin_unlock(swap_lock);
inode = mapping-host;
/* If S_ISREG(inode-i_mode) will do mutex_lock(inode-i_mutex); */
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
