I unlinked a swapfile without realizing I was still swapping on it.
Now my /proc/swaps looks like this:
Filename Type Size Used Priority
/var/tmp/EXP/cache/swap/1\040(deleted) file 1288644 1448 -1
/var/tmp/EXP/cache/swap/2\040(deleted) file 1433368 0 -2
AFAICT, there's nothing I can pass to swapoff(2) that will make the
kernel let go of them. If that's the case, please consider this a
feature request for a way to do this. Now I'm going to have to reboot
before I can mkfs that partition.
If kswapd0 had a fd open on the swap files, swapoff /proc/$PID/fd/3
could possibly work. But it looks like the files are open but with no
user-space accessable file descriptors to them. Which makes sense,
except for this case.
thanks, and happy hacking,
--
#define X(x,y) x##y
Peter Cordes ; e-mail: X(peter@cor , des.ca)
"The gods confound the man who first found out how to distinguish the hours!
Confound him, too, who in this place set up a sundial, to cut and hack
my day so wretchedly into small pieces!" -- Plautus, 200 BC
On Wed, 2008-10-15 at 17:21 -0300, Peter Cordes wrote:
> I unlinked a swapfile without realizing I was still swapping on it.
> Now my /proc/swaps looks like this:
> Filename Type Size Used Priority
> /var/tmp/EXP/cache/swap/1\040(deleted) file 1288644 1448 -1
> /var/tmp/EXP/cache/swap/2\040(deleted) file 1433368 0 -2
>
> AFAICT, there's nothing I can pass to swapoff(2) that will make the
> kernel let go of them. If that's the case, please consider this a
> feature request for a way to do this. Now I'm going to have to reboot
> before I can mkfs that partition.
>
> If kswapd0 had a fd open on the swap files, swapoff /proc/$PID/fd/3
> could possibly work. But it looks like the files are open but with no
> user-space accessable file descriptors to them. Which makes sense,
> except for this case.
Right, except that kswapd is per node, so we'd either have to add it to
all kswapd instances or a random one. Also, kthreads don't seem to have
a files table afaict.
But yes, I see your problem and it makes sense to look for a nice
solution.
On Thu, 16 Oct 2008, Peter Zijlstra wrote:
> On Wed, 2008-10-15 at 17:21 -0300, Peter Cordes wrote:
> > I unlinked a swapfile without realizing I was still swapping on it.
> > Now my /proc/swaps looks like this:
> > Filename Type Size Used Priority
> > /var/tmp/EXP/cache/swap/1\040(deleted) file 1288644 1448 -1
> > /var/tmp/EXP/cache/swap/2\040(deleted) file 1433368 0 -2
> >
> > AFAICT, there's nothing I can pass to swapoff(2) that will make the
> > kernel let go of them. If that's the case, please consider this a
> > feature request for a way to do this. Now I'm going to have to reboot
> > before I can mkfs that partition.
> >
> > If kswapd0 had a fd open on the swap files, swapoff /proc/$PID/fd/3
> > could possibly work. But it looks like the files are open but with no
> > user-space accessable file descriptors to them. Which makes sense,
> > except for this case.
>
> Right, except that kswapd is per node, so we'd either have to add it to
> all kswapd instances or a random one. Also, kthreads don't seem to have
> a files table afaict.
>
> But yes, I see your problem and it makes sense to look for a nice
> solution.
No immediate answer springs to my mind.
It's not something I'd want to add a new system call for.
I guess we could put a magic file for each swap area
somewhere down in /sys, and allow swapoff to act upon that.
If there were other good reasons to add such files, that
could make sense. But although I'll willingly admit it's a
lacuna, I don't think it's one worth bloating the kernel for.
(I would suggest that Peter keep a second link to his swapfiles
somewhere safer; but that's then open to the converse complaint,
that when he unlinks intentionally but forgets the safe link,
the disk space remains mysteriously in use.)
Sorry for being unhelpful!
Hugh
Hugh Dickins <[email protected]> wrote:
> On Thu, 16 Oct 2008, Peter Zijlstra wrote:
>> On Wed, 2008-10-15 at 17:21 -0300, Peter Cordes wrote:
>> > I unlinked a swapfile without realizing I was still swapping on it.
>> > Now my /proc/swaps looks like this:
>> > Filename Type Size Used
>> > Priority
>> > /var/tmp/EXP/cache/swap/1\040(deleted) file 1288644 1448 -1
>> > /var/tmp/EXP/cache/swap/2\040(deleted) file 1433368 0 -2
>> > If kswapd0 had a fd open on the swap files, swapoff /proc/$PID/fd/3
>> > could possibly work. But it looks like the files are open but with no
>> > user-space accessable file descriptors to them. Which makes sense,
>> > except for this case.
>>
>> Right, except that kswapd is per node, so we'd either have to add it to
>> all kswapd instances or a random one. Also, kthreads don't seem to have
>> a files table afaict.
>>
>> But yes, I see your problem and it makes sense to look for a nice
>> solution.
>
> No immediate answer springs to my mind.
>
> It's not something I'd want to add a new system call for.
> I guess we could put a magic file for each swap area
> somewhere down in /sys, and allow swapoff to act upon that.
I think the original idea of something like /proc/$PID/fd/ is not too bad.
I don't know if it's possible to have the same mechanism in sysfs. I guess
not, but with the rest of the vm knobs being in /proc, I would not be too sad.
Maybe it's possible to clone(CLONE_FILES) the kswapds. This would allow to
have /proc/sys/vm/swapfiles point to one of the correct /proc/$kwapd/fd/.
Hugh Dickins wrote:
> On Thu, 16 Oct 2008, Peter Zijlstra wrote:
>
>> On Wed, 2008-10-15 at 17:21 -0300, Peter Cordes wrote:
>>
>>> I unlinked a swapfile without realizing I was still swapping on it.
>>>
>> I see your problem and it makes sense to look for a nice solution.
>>
>
> although I'll willingly admit it's a
> lacuna, I don't think it's one worth bloating the kernel for.
Me too. The kernel shouldn't protect the administrator against all
possible mistakes; and this mistake is one of them. Besides, who's to
say it's always a mistake? Somebody might want their swap file to have
zero links.
Do nothing.
David Newall <[email protected]> wrote:
> Hugh Dickins wrote:
>> On Thu, 16 Oct 2008, Peter Zijlstra wrote:
>>> On Wed, 2008-10-15 at 17:21 -0300, Peter Cordes wrote:
>>>
>>>> I unlinked a swapfile without realizing I was still swapping on it.
[...]
> Me too. The kernel shouldn't protect the administrator against all
> possible mistakes; and this mistake is one of them. Besides, who's to
> say it's always a mistake? Somebody might want their swap file to have
> zero links.
Somebody might want their swapfiles to have zero links, _and_ the possibility
of doing swapoff. If you can do it by keeping some fds open to let
/proc/pid/fd point to the files, I think it's OK.
On Fri, 17 Oct 2008, Bodo Eggert wrote:
>
> Somebody might want their swapfiles to have zero links,
> _and_ the possibility of doing swapoff.
You're right, they might, and it's not an unreasonable wish.
But we've not supported it in the past, and I still don't
think it's worth adding special kernel support for it now.
> If you can do it by keeping some fds open to let
> /proc/pid/fd point to the files, I think it's OK.
I've a very strong aversion to adding strange code to abuse the
/proc/<pid>/fd space of some random kernel thread - a "kswapd"
because its name contains the substring "swap"?
Hugh
Hugh Dickins wrote:
> On Fri, 17 Oct 2008, Bodo Eggert wrote:
>
>> Somebody might want their swapfiles to have zero links,
>> _and_ the possibility of doing swapoff.
>>
>
> You're right, they might, and it's not an unreasonable wish.
> But we've not supported it in the past, and I still don't
> think it's worth adding special kernel support for it now.
But it is supported now. It's swapoff that's not supported, and I don't
think that matters.
On Fri, 17 Oct 2008, Hugh Dickins wrote:
> On Fri, 17 Oct 2008, Bodo Eggert wrote:
> > Somebody might want their swapfiles to have zero links,
> > _and_ the possibility of doing swapoff.
>
> You're right, they might, and it's not an unreasonable wish.
> But we've not supported it in the past, and I still don't
> think it's worth adding special kernel support for it now.
IMO it depends on the cost. Maybe it's cheap to keep an extra fd around,
maybe you'd have to add an extra infrastructure for this. And maybe it's not
important enough for anybody to create a patch and let us know ...
--
Funny quotes:
9. Despite the cost of living, have you noticed how popular it remains?
On Fri, Oct 17, 2008 at 01:17:17PM +0100, Hugh Dickins wrote:
> On Fri, 17 Oct 2008, Bodo Eggert wrote:
> >
> > Somebody might want their swapfiles to have zero links,
> > _and_ the possibility of doing swapoff.
>
> You're right, they might, and it's not an unreasonable wish.
> But we've not supported it in the past, and I still don't
> think it's worth adding special kernel support for it now.
I'd be inclined to agree with not bloating the kernel to support
this, even though it would have been convenient for me in one case. I
do have an idea for supporting this without bloat, see below. In case
anyone wants more details about how I painted myself into that corner,
here's the backstory to my feature request.
I was cleaning out /var/tmp until I had it down to everything I
wanted to keep. Then I was going to rsync it to somewhere else,
umount, mkfs (with a newer mkfs.xfs -l version=2,lazy-count=1 -n
version=2, etc...), and copy my files back. I think I forgot to
/etc/init.d/swapspace stop before rm -r on my swapfile directory.
I run swapspace(8), http://pqxx.org/development/swapspace
http://packages.debian.org/sid/swapspace. I have a ~700MB swap
partition that will actually get used most of the time, and let
swapspace(8) make swap files in /var/cache/swapspace. (I symlink or
bind mount /var/cache so it's actually on /var/tmp, since it seems to
fit better there.)
Usually dynamic swap file creation just delays the inevitable OOM and
makes thrashing worse. (because one can't create swap files without
writing them full of zeros, not even with xfs_io resvsp, except with
old XFS without unwritten extent tracking. But that's another feature
request...) In the rare case where you're running a few bloated things
and want to let them swap out while doing something unusual that takes
almost all your RAM, it's fairly convenient. Basically I like the
idea, even if in practice it's clunky and would be better to just use
some big swap files I could delete if I need the disk space. It works
well when your swap isn't filling up as fast as your disk can write.
> > If you can do it by keeping some fds open to let
> > /proc/pid/fd point to the files, I think it's OK.
>
> I've a very strong aversion to adding strange code to abuse the
> /proc/<pid>/fd space of some random kernel thread - a "kswapd"
> because its name contains the substring "swap"?
Yes, because it has swap in the name :P. I was just guessing it had
something to do with swap areas.
Here's another idea: swapoff(2) takes a text string. It could be
overloaded to parse the string as a numeric index into the list of
swap areas (as listed in /proc/swaps). It could do this as a fallback
iff the string was numeric and didn't exist as a pathname relative to
the CWD. (You don't want "/nonexistant/file" to be treated the same as
"0" of course, or you'd always be swapoff()ing the first swap file
instead of returning ENOENT.)
Before rebooting I tried
# swapoff "/var/tmp/EXP/cache/swap/1 (deleted)"
Then I looked at the source to see that that sys_swapoff only tries
to use the string as a path in the filesystem. But that doesn't have
to be the case, if there's enough information in data structures
associated with a swap area to get whatever swapoff() needs to let go
of the file.
This would just add some kernel code that wouldn't usually be in the
CPU icache, and wouldn't allocate any extra objects like file
descriptors or VFS entries to lie around to support this.
It's a little ugly, though, to change a system call that normally
accepts only paths to accept things that aren't paths. It would be
even worse to use swapoff("/sys/swaps/2") if /sys/swaps didn't exist
in the VFS, and was parsed specially by sys_swapoff(). So don't do
that. :) swapoff("0") doesn't seem too bad, if only by comparison
with something horrible.
--
#define X(x,y) x##y
Peter Cordes ; e-mail: X(peter@cor , des.ca)
"The gods confound the man who first found out how to distinguish the hours!
Confound him, too, who in this place set up a sundial, to cut and hack
my day so wretchedly into small pieces!" -- Plautus, 200 BC
On Fri, Oct 17, 2008 at 09:31:17PM -0300, Peter Cordes wrote:
> On Fri, Oct 17, 2008 at 01:17:17PM +0100, Hugh Dickins wrote:
> > On Fri, 17 Oct 2008, Bodo Eggert wrote:
> > >
> > > Somebody might want their swapfiles to have zero links,
> > > _and_ the possibility of doing swapoff.
> >
> > You're right, they might, and it's not an unreasonable wish.
> > But we've not supported it in the past, and I still don't
> > think it's worth adding special kernel support for it now.
>
> I'd be inclined to agree with not bloating the kernel to support
> this, even though it would have been convenient for me in one case. I
> do have an idea for supporting this without bloat, see below. In case
> anyone wants more details about how I painted myself into that corner,
> here's the backstory to my feature request.
(...)
I have another idea which might be simpler to implement in userspace.
What happened to you is a typical accident, you did not run on purpose
on a deleted swap file. So we should at least ensure that such types
of accidents could not happen easily.
If swapon did set the immutable bit on a file just after enabling swap
to it, it would at least prevent accidental removal of that file. Swapoff
would have to clean that bit, and swapon would have to clean it upon
startup too (in case of unplanned reboots).
That way, you could still remove such files on purpose provided you do
a preliminary "chattr -i" on them, but "rm -rf" would keep them intact.
It would also prevent accidental modifications, such as "ls .>swapfile"
instead of "ls ./swapfile".
Regards,
Willy
On Sat, 18 Oct 2008, Willy Tarreau wrote:
> (...)
> I have another idea which might be simpler to implement in userspace.
> What happened to you is a typical accident, you did not run on purpose
> on a deleted swap file. So we should at least ensure that such types
> of accidents could not happen easily.
>
> If swapon did set the immutable bit on a file just after enabling swap
> to it, it would at least prevent accidental removal of that file. Swapoff
> would have to clean that bit, and swapon would have to clean it upon
> startup too (in case of unplanned reboots).
>
> That way, you could still remove such files on purpose provided you do
> a preliminary "chattr -i" on them, but "rm -rf" would keep them intact.
That's a good idea, thank you Willy:
much more to my taste than previous suggestions.
But I'm still not tempted to build it into the swapon and swapoff,
neither at the command nor at the kernel level. Let's leave it as
advice to sufferers on how to address the issue if it troubles them.
I did play with immutable on swapfiles back around 2.6.8. Prior
to that we left i_sem downed on a swapfile to protect it against
truncation (freeing its pages!) while in use - which caused
anyone idly touching it to hang, not very nice.
I experimented with setting immutable in sys_swapon, clearing it
in sys_swapoff, but I see from old mails that I didn't find that
satisfactory: I haven't actually recorded why not, but I think it
was partly a difficulty in getting the locking right, and partly
what happened if the user also made it immutable while swapped on -
oh, yes, and immutable gets written back to the filesystem which is
inconvenient when we crash, as you observe. So we ended up adding
an additional swapfile flag just at the VFS level.
Hmm, I suppose it would be very easy to make that additional swapfile
flag prohibit unlinking just as immutable does: patch below should do
that. What do you guys think - should we include this? It does then
(barring races which I don't propose to worry about) make an unlinked
swapfile impossible, which earlier had seemed a reasonable option.
> It would also prevent accidental modifications, such as "ls .>swapfile"
> instead of "ls ./swapfile".
That we do already protect against with the swapfile flag: we don't
actually protect against writing (that's just a permission thing,
same as when swapping to block device), but we do protect against
truncation, which would otherwise end up with swap corrupting
blocks of other files.
Hugh
--- 2.6.27/fs/namei.c 2008-10-09 23:13:53.000000000 +0100
+++ linux/fs/namei.c 2008-10-18 21:33:01.000000000 +0100
@@ -1407,7 +1407,7 @@ static int may_delete(struct inode *dir,
if (IS_APPEND(dir))
return -EPERM;
if (check_sticky(dir, victim->d_inode)||IS_APPEND(victim->d_inode)||
- IS_IMMUTABLE(victim->d_inode))
+ IS_IMMUTABLE(victim->d_inode) || IS_SWAPFILE(victim->d_inode))
return -EPERM;
if (isdir) {
if (!S_ISDIR(victim->d_inode->i_mode))
On Sat, Oct 18, 2008 at 09:45:14PM +0100, Hugh Dickins wrote:
> --- 2.6.27/fs/namei.c 2008-10-09 23:13:53.000000000 +0100
> +++ linux/fs/namei.c 2008-10-18 21:33:01.000000000 +0100
> @@ -1407,7 +1407,7 @@ static int may_delete(struct inode *dir,
> if (IS_APPEND(dir))
> return -EPERM;
> if (check_sticky(dir, victim->d_inode)||IS_APPEND(victim->d_inode)||
> - IS_IMMUTABLE(victim->d_inode))
> + IS_IMMUTABLE(victim->d_inode) || IS_SWAPFILE(victim->d_inode))
> return -EPERM;
> if (isdir) {
> if (!S_ISDIR(victim->d_inode->i_mode))
Looks reasonable.
On Sat, Oct 18, 2008 at 04:49:48PM -0400, Christoph Hellwig wrote:
> On Sat, Oct 18, 2008 at 09:45:14PM +0100, Hugh Dickins wrote:
> > --- 2.6.27/fs/namei.c 2008-10-09 23:13:53.000000000 +0100
> > +++ linux/fs/namei.c 2008-10-18 21:33:01.000000000 +0100
> > @@ -1407,7 +1407,7 @@ static int may_delete(struct inode *dir,
> > if (IS_APPEND(dir))
> > return -EPERM;
> > if (check_sticky(dir, victim->d_inode)||IS_APPEND(victim->d_inode)||
> > - IS_IMMUTABLE(victim->d_inode))
> > + IS_IMMUTABLE(victim->d_inode) || IS_SWAPFILE(victim->d_inode))
> > return -EPERM;
> > if (isdir) {
> > if (!S_ISDIR(victim->d_inode->i_mode))
>
> Looks reasonable.
I like the idea and the simplicity a lot !
Willy
Peter Cordes is sorry that he rm'ed his swapfiles while they were in use,
he then had no pathname to swapoff. It's a curious little oversight, but
not one worth a lot of hackery. Kudos to Willy Tarreau for turning this
around from a discussion of synthetic pathnames to how to prevent unlink.
Mimic immutable: prohibit unlinking an active swapfile in may_delete()
(and don't worry my little head over the tiny race window).
Signed-off-by: Hugh Dickins <[email protected]>
---
Perhaps this is too late for 2.6.28: your decision.
fs/namei.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- 2.6.28-rc4/fs/namei.c 2008-10-24 09:28:19.000000000 +0100
+++ linux/fs/namei.c 2008-11-12 11:52:44.000000000 +0000
@@ -1378,7 +1378,7 @@ static int may_delete(struct inode *dir,
if (IS_APPEND(dir))
return -EPERM;
if (check_sticky(dir, victim->d_inode)||IS_APPEND(victim->d_inode)||
- IS_IMMUTABLE(victim->d_inode))
+ IS_IMMUTABLE(victim->d_inode) || IS_SWAPFILE(victim->d_inode))
return -EPERM;
if (isdir) {
if (!S_ISDIR(victim->d_inode->i_mode))
On Fri, Nov 14, 2008 at 02:37:22AM +0000, Hugh Dickins wrote:
> Peter Cordes is sorry that he rm'ed his swapfiles while they were in use,
> he then had no pathname to swapoff. It's a curious little oversight, but
> not one worth a lot of hackery.
Yeah, not a lot, but I'd say it's worth some. On the system where I
'rm -rf'ed part of a filesystem before cp -a, mkfs, cp -a, I was left
unable to umount. Plus, when I rebooted, Ubuntu's init scripts failed
to even sync the disks during shutdown. A recently-written file on
the same XFS filesystem as the swap file ended up empty because of the
unclean shutdown. :( I don't know if remount ro would have been
possible on a FS with an active swap file, but Ubuntu should have at
least tried to sync when umount failed.
> Kudos to Willy Tarreau for turning this
> around from a discussion of synthetic pathnames to how to prevent unlink.
Yeah, this is great; as a sysadmin, this produces exactly the right
behaviour, IMHO. It doesn't have any chance of leaving files marked
immutable on disk after an unclean reboot, which was a fatal flaw in
the idea of setting the i bit on swap files, either in swapon(8) or in
the kernel. That would introduce complexity for admins who would
otherwise never have to think about this. The new behaviour this adds
should make sense to most admins; They'll see
rm: swapfile: permission denied
or something, and should quickly realize that there must be something
special about active swap files. So it's discoverable (i.e.
non-mysterious) behaviour.
This prevents running with a deleted swapfile, but I can't think of a
case when that's useful, let alone worth the trouble of writing a new one every
reboot. (e.g. xfs's resvsp ioctl creates extents flagged as unwritten
which can't be swapped on, so a swapfile would have to be actually
written on most filesystems.)
And it doesn't add any size to the kernel binary, unlike my idea of
having a /proc/something/fd link that one could swapoff, having
sys_swapoff() fall back to parsing its argument as an integer index
into a list of swap areas, or other ugly ideas... :P
Thanks everyone for coming up with such a clever solution to the
pitfall I found.
> Mimic immutable: prohibit unlinking an active swapfile in may_delete()
> (and don't worry my little head over the tiny race window).
>
> Signed-off-by: Hugh Dickins <[email protected]>
> ---
> Perhaps this is too late for 2.6.28: your decision.
>
> fs/namei.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- 2.6.28-rc4/fs/namei.c 2008-10-24 09:28:19.000000000 +0100
> +++ linux/fs/namei.c 2008-11-12 11:52:44.000000000 +0000
> @@ -1378,7 +1378,7 @@ static int may_delete(struct inode *dir,
> if (IS_APPEND(dir))
> return -EPERM;
> if (check_sticky(dir, victim->d_inode)||IS_APPEND(victim->d_inode)||
> - IS_IMMUTABLE(victim->d_inode))
> + IS_IMMUTABLE(victim->d_inode) || IS_SWAPFILE(victim->d_inode))
> return -EPERM;
> if (isdir) {
> if (!S_ISDIR(victim->d_inode->i_mode))
--
#define X(x,y) x##y
Peter Cordes ; e-mail: X(peter@cor , des.ca)
"The gods confound the man who first found out how to distinguish the hours!
Confound him, too, who in this place set up a sundial, to cut and hack
my day so wretchedly into small pieces!" -- Plautus, 200 BC
On Fri, Nov 14, 2008 at 02:37:22AM +0000, Hugh Dickins wrote:
> Peter Cordes is sorry that he rm'ed his swapfiles while they were in use,
> he then had no pathname to swapoff. It's a curious little oversight, but
> not one worth a lot of hackery. Kudos to Willy Tarreau for turning this
> around from a discussion of synthetic pathnames to how to prevent unlink.
> Mimic immutable: prohibit unlinking an active swapfile in may_delete()
> (and don't worry my little head over the tiny race window).
Looks good (but I think I already said this before)
On Fri, 14 Nov 2008, Christoph Hellwig wrote:
> On Fri, Nov 14, 2008 at 02:37:22AM +0000, Hugh Dickins wrote:
> > Peter Cordes is sorry that he rm'ed his swapfiles while they were in use,
> > he then had no pathname to swapoff. It's a curious little oversight, but
> > not one worth a lot of hackery. Kudos to Willy Tarreau for turning this
> > around from a discussion of synthetic pathnames to how to prevent unlink.
> > Mimic immutable: prohibit unlinking an active swapfile in may_delete()
> > (and don't worry my little head over the tiny race window).
>
> Looks good (but I think I already said this before)
Thanks. Indeed you did. I wondered whether to add an Acked-by in
your name, but I think different people have understandably different
positions on the etiquette of translating "Looks good" into "Acked-by";
so not knowing your position, I just Cc'ed you again.
Hugh