Linux Btrfs filesystem development
 help / color / mirror / Atom feed
* [RFC] New ioctl to query deleted subvolumes
@ 2024-10-09 18:03 David Sterba
  2024-10-10 12:29 ` Yuwei Han
  0 siblings, 1 reply; 7+ messages in thread
From: David Sterba @ 2024-10-09 18:03 UTC (permalink / raw)
  To: linux-btrfs

Hi,

I'd like some user feedback on a new ioctl that should handle several use cases
around subvolume deletion. Currently it's implemented on top of the privileged
SEARCH_TREE ioctl, it's not possible to do that with libbtrfsutil or
unprivileged ioctls.

The use cases:

  1) wait for a specific id until it's cleaned (blocking, not blocking)

This is what 'btrfs subvol sync id' does. In the non-blocking mode it checks if
a subvolume is in the queue for deletion.

  2) wait for all currently queued subvolumes (blocking)

Same as 'btrfs subvol sync' without any id.

  3) read id of the currently cleaned subvolume (not blocking)

Allow to implement sync purely in user space.

  4) read id of the last in the queue (not blocking)

As the subvolumes are added to the list in the order of deletion, reading the
last one is kind of a checkpoint. More subvolumes can be added to the queue in
the meanwhile so this adds some flexibility to applications.

  5) count the number of queued subvolumes (not blocking)

This is for convenience and progress reports.


There are some questions and potential problems stemming from the general
availability of the ioctl:

- the operations will need to take locks and/or lookup the subvolumes in the
  data structures, so it could be abused to overload the locks, but there are
  more such ways how to do that so I'm not sure what to do here

- deleted subvolume loses it's connection to path in directory hierarchy, so
  querying an id does not tell us if the user was allowed to see the subvolume

- the blocking operations can take a timeout parameter (seconds), this is for
  convenience, otherwise a simple "while (ioctl) sleep(1)" will always work


My questions:

- Are there other use cases that are missing from the list?

- Are there use cases that should not be implemented? E.g. not worth the
  trouble or not really useful.

I have a prototype for 1 and 2, the others would be easy to implement
but the number of cases affects the ioctl design (simple id vs modes).

Thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] New ioctl to query deleted subvolumes
  2024-10-09 18:03 [RFC] New ioctl to query deleted subvolumes David Sterba
@ 2024-10-10 12:29 ` Yuwei Han
  2024-10-10 17:08   ` David Sterba
  0 siblings, 1 reply; 7+ messages in thread
From: Yuwei Han @ 2024-10-10 12:29 UTC (permalink / raw)
  To: dsterba, linux-btrfs



在 2024/10/10 02:03, David Sterba 写道:
> Hi,
> 
> I'd like some user feedback on a new ioctl that should handle several use cases
> around subvolume deletion. Currently it's implemented on top of the privileged
> SEARCH_TREE ioctl, it's not possible to do that with libbtrfsutil or
> unprivileged ioctls.
> 
> The use cases:
> 
>    1) wait for a specific id until it's cleaned (blocking, not blocking)
> 
> This is what 'btrfs subvol sync id' does. In the non-blocking mode it checks if
> a subvolume is in the queue for deletion.
> 
>    2) wait for all currently queued subvolumes (blocking)
> 
> Same as 'btrfs subvol sync' without any id.
> 
>    3) read id of the currently cleaned subvolume (not blocking)
> 
> Allow to implement sync purely in user space.
> 
>    4) read id of the last in the queue (not blocking)
> 
> As the subvolumes are added to the list in the order of deletion, reading the
> last one is kind of a checkpoint. More subvolumes can be added to the queue in
> the meanwhile so this adds some flexibility to applications.
> 
>    5) count the number of queued subvolumes (not blocking)
> 
> This is for convenience and progress reports.
I don't understand why we need to get the status about subvolume 
deletion. Can you provide some real world usage?
> 
> There are some questions and potential problems stemming from the general
> availability of the ioctl:
> 
> - the operations will need to take locks and/or lookup the subvolumes in the
>    data structures, so it could be abused to overload the locks, but there are
>    more such ways how to do that so I'm not sure what to do here
Since it's privileged operation. I think it is up to users. But we can 
have some hints like balance do.
> - deleted subvolume loses it's connection to path in directory hierarchy, so
>    querying an id does not tell us if the user was allowed to see the subvolume
> 
> - the blocking operations can take a timeout parameter (seconds), this is for
>    convenience, otherwise a simple "while (ioctl) sleep(1)" will always work
> 
> 
> My questions:
> 
> - Are there other use cases that are missing from the list?
> 
> - Are there use cases that should not be implemented? E.g. not worth the
>    trouble or not really useful.
> 
> I have a prototype for 1 and 2, the others would be easy to implement
> but the number of cases affects the ioctl design (simple id vs modes).
> 
> Thanks.
> 
Overall I am confused about this message. Did I miss something before?

HAN Yuwei


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] New ioctl to query deleted subvolumes
  2024-10-10 12:29 ` Yuwei Han
@ 2024-10-10 17:08   ` David Sterba
  2024-10-11 12:06     ` Yuwei Han
  0 siblings, 1 reply; 7+ messages in thread
From: David Sterba @ 2024-10-10 17:08 UTC (permalink / raw)
  To: Yuwei Han; +Cc: dsterba, linux-btrfs

On Thu, Oct 10, 2024 at 08:29:14PM +0800, Yuwei Han wrote:
> 
> 
> 在 2024/10/10 02:03, David Sterba 写道:
> > Hi,
> > 
> > I'd like some user feedback on a new ioctl that should handle several use cases
> > around subvolume deletion. Currently it's implemented on top of the privileged
> > SEARCH_TREE ioctl, it's not possible to do that with libbtrfsutil or
> > unprivileged ioctls.
> > 
> > The use cases:
> > 
> >    1) wait for a specific id until it's cleaned (blocking, not blocking)
> > 
> > This is what 'btrfs subvol sync id' does. In the non-blocking mode it checks if
> > a subvolume is in the queue for deletion.
> > 
> >    2) wait for all currently queued subvolumes (blocking)
> > 
> > Same as 'btrfs subvol sync' without any id.
> > 
> >    3) read id of the currently cleaned subvolume (not blocking)
> > 
> > Allow to implement sync purely in user space.
> > 
> >    4) read id of the last in the queue (not blocking)
> > 
> > As the subvolumes are added to the list in the order of deletion, reading the
> > last one is kind of a checkpoint. More subvolumes can be added to the queue in
> > the meanwhile so this adds some flexibility to applications.
> > 
> >    5) count the number of queued subvolumes (not blocking)
> > 
> > This is for convenience and progress reports.

> I don't understand why we need to get the status about subvolume 
> deletion. Can you provide some real world usage?

https://github.com/kdave/btrfs-progs/blob/devel/cmds/subvolume.c#L85

'btrfs subvol sync' prints at the beginning number of subvolumes

  "Waiting for 123 subvolumes"

And then prints the progress like

  "Subvolume id 456 is gone (1/10)"

So the ioctl mode is there to emulate that, for the common case of "wait
for all subvolumes currently queued for deletion".

> > 
> > There are some questions and potential problems stemming from the general
> > availability of the ioctl:
> > 
> > - the operations will need to take locks and/or lookup the subvolumes in the
> >    data structures, so it could be abused to overload the locks, but there are
> >    more such ways how to do that so I'm not sure what to do here
> Since it's privileged operation. I think it is up to users. But we can 
> have some hints like balance do.
> > - deleted subvolume loses it's connection to path in directory hierarchy, so
> >    querying an id does not tell us if the user was allowed to see the subvolume
> > 
> > - the blocking operations can take a timeout parameter (seconds), this is for
> >    convenience, otherwise a simple "while (ioctl) sleep(1)" will always work
> > 
> > 
> > My questions:
> > 
> > - Are there other use cases that are missing from the list?
> > 
> > - Are there use cases that should not be implemented? E.g. not worth the
> >    trouble or not really useful.
> > 
> > I have a prototype for 1 and 2, the others would be easy to implement
> > but the number of cases affects the ioctl design (simple id vs modes).
> > 
> > Thanks.
> > 
> Overall I am confused about this message. Did I miss something before?

Sorry, without the code it could be cryptic.  In the simplest version,
the ioctl could be defined to take only the a u64, the id of the
subvolume to wait for, or 0 to wait for all.

#define BTRFS_IOC_SUBVOL_SYNC_WAIT _IOW(BTRFS_IOCTL_MAGIC, 65, u64)

and implemented like (schematically):

btrfs_ioctl_subvol_sync_wait(struct btrfs_fs_info *fs_info, void __user *argp) {
	u64 subvolid;

	copy_from_user(&subvolid, argp);

	if (subvolid == 0)
		subvolid = btrfs_root_id("last root in the dead_roots list");

	while (1) {
		root = lookup_root(subvolid)
		"if exists, continue looping"
		"if gone, exit"
	}
}

So my question is if this is sufficient for all what's needed or if the
argument passed to the ioctl should be a structure with flags, modes,
timeout and such:

struct btrfs_ioctl_subvol_wailt {
	u32 mode;
	union {
		u32 timeout;
		u32 count;
	};
	u64 subvolid;
};

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] New ioctl to query deleted subvolumes
  2024-10-10 17:08   ` David Sterba
@ 2024-10-11 12:06     ` Yuwei Han
  2024-10-11 12:20       ` Roman Mamedov
  2024-10-11 15:48       ` David Sterba
  0 siblings, 2 replies; 7+ messages in thread
From: Yuwei Han @ 2024-10-11 12:06 UTC (permalink / raw)
  To: dsterba; +Cc: linux-btrfs



在 2024/10/11 01:08, David Sterba 写道:
> On Thu, Oct 10, 2024 at 08:29:14PM +0800, Yuwei Han wrote:
>>
>>
>> 在 2024/10/10 02:03, David Sterba 写道:
>>> Hi,
>>>
>>> I'd like some user feedback on a new ioctl that should handle several use cases
>>> around subvolume deletion. Currently it's implemented on top of the privileged
>>> SEARCH_TREE ioctl, it's not possible to do that with libbtrfsutil or
>>> unprivileged ioctls.
>>>
>>> The use cases:
>>>
>>>     1) wait for a specific id until it's cleaned (blocking, not blocking)
>>>
>>> This is what 'btrfs subvol sync id' does. In the non-blocking mode it checks if
>>> a subvolume is in the queue for deletion.
>>>
>>>     2) wait for all currently queued subvolumes (blocking)
>>>
>>> Same as 'btrfs subvol sync' without any id.
>>>
>>>     3) read id of the currently cleaned subvolume (not blocking)
>>>
>>> Allow to implement sync purely in user space.
>>>
>>>     4) read id of the last in the queue (not blocking)
>>>
>>> As the subvolumes are added to the list in the order of deletion, reading the
>>> last one is kind of a checkpoint. More subvolumes can be added to the queue in
>>> the meanwhile so this adds some flexibility to applications.
>>>
>>>     5) count the number of queued subvolumes (not blocking)
>>>
>>> This is for convenience and progress reports.
> 
>> I don't understand why we need to get the status about subvolume
>> deletion. Can you provide some real world usage?
> 
> https://github.com/kdave/btrfs-progs/blob/devel/cmds/subvolume.c#L85
> 
> 'btrfs subvol sync' prints at the beginning number of subvolumes
> 
>    "Waiting for 123 subvolumes"
> 
> And then prints the progress like
> 
>    "Subvolume id 456 is gone (1/10)"
> 
> So the ioctl mode is there to emulate that, for the common case of "wait
> for all subvolumes currently queued for deletion".
> 
There is a misunderstanding. What I mean is that why users need to 
"confirm" subvol is deleted? I can't come up with actual usage. Hope you 
can help me with that.
>>>
>>> There are some questions and potential problems stemming from the general
>>> availability of the ioctl:
>>>
>>> - the operations will need to take locks and/or lookup the subvolumes in the
>>>     data structures, so it could be abused to overload the locks, but there are
>>>     more such ways how to do that so I'm not sure what to do here
>> Since it's privileged operation. I think it is up to users. But we can
>> have some hints like balance do.
>>> - deleted subvolume loses it's connection to path in directory hierarchy, so
>>>     querying an id does not tell us if the user was allowed to see the subvolume
>>>
>>> - the blocking operations can take a timeout parameter (seconds), this is for
>>>     convenience, otherwise a simple "while (ioctl) sleep(1)" will always work
>>>
>>>
>>> My questions:
>>>
>>> - Are there other use cases that are missing from the list?
>>>
>>> - Are there use cases that should not be implemented? E.g. not worth the
>>>     trouble or not really useful.
>>>
>>> I have a prototype for 1 and 2, the others would be easy to implement
>>> but the number of cases affects the ioctl design (simple id vs modes).
>>>
>>> Thanks.
>>>
>> Overall I am confused about this message. Did I miss something before?
> 
> Sorry, without the code it could be cryptic.  In the simplest version,
> the ioctl could be defined to take only the a u64, the id of the
> subvolume to wait for, or 0 to wait for all.
> 
> #define BTRFS_IOC_SUBVOL_SYNC_WAIT _IOW(BTRFS_IOCTL_MAGIC, 65, u64)
> 
> and implemented like (schematically):
> 
> btrfs_ioctl_subvol_sync_wait(struct btrfs_fs_info *fs_info, void __user *argp) {
> 	u64 subvolid;
> 
> 	copy_from_user(&subvolid, argp);
> 
> 	if (subvolid == 0)
> 		subvolid = btrfs_root_id("last root in the dead_roots list");
> 
user may delete more subvol after sync wait. Is this ok?
> 	while (1) {
> 		root = lookup_root(subvolid)
> 		"if exists, continue looping"
> 		"if gone, exit"
> 	}
> }
> 
> So my question is if this is sufficient for all what's needed or if the
> argument passed to the ioctl should be a structure with flags, modes,
> timeout and such:
> 
> struct btrfs_ioctl_subvol_wailt {
> 	u32 mode;
> 	union {
> 		u32 timeout;
> 		u32 count;
> 	};
> 	u64 subvolid;
> };
> 
Seems good to me. I have no more suggestions.

HAN Yuwei

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] New ioctl to query deleted subvolumes
  2024-10-11 12:06     ` Yuwei Han
@ 2024-10-11 12:20       ` Roman Mamedov
  2024-10-12  1:45         ` Yuwei Han
  2024-10-11 15:48       ` David Sterba
  1 sibling, 1 reply; 7+ messages in thread
From: Roman Mamedov @ 2024-10-11 12:20 UTC (permalink / raw)
  To: Yuwei Han; +Cc: dsterba, linux-btrfs

On Fri, 11 Oct 2024 20:06:00 +0800
Yuwei Han <hrx@bupt.moe> wrote:

> There is a misunderstanding. What I mean is that why users need to 
> "confirm" subvol is deleted? I can't come up with actual usage. Hope you 
> can help me with that.

A backup system may delete outdated subvolumes first, then wait until the
space has actually freed up, before copying new data; otherwise it could run
out of disk space. Especially considering that IO from starting to copy
without such a wait would interfere with the cleaner process IO, resulting in
it taking a longer time to finish.

Or in my case I also wait until deleted subvolumes have been freed up *after*
a backup, to then record how much remaining space a backup disk actually has,
before unmounting it and putting into storage.

-- 
With respect,
Roman

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] New ioctl to query deleted subvolumes
  2024-10-11 12:06     ` Yuwei Han
  2024-10-11 12:20       ` Roman Mamedov
@ 2024-10-11 15:48       ` David Sterba
  1 sibling, 0 replies; 7+ messages in thread
From: David Sterba @ 2024-10-11 15:48 UTC (permalink / raw)
  To: Yuwei Han; +Cc: dsterba, linux-btrfs

On Fri, Oct 11, 2024 at 08:06:00PM +0800, Yuwei Han wrote:
> > So the ioctl mode is there to emulate that, for the common case of "wait
> > for all subvolumes currently queued for deletion".

> There is a misunderstanding. What I mean is that why users need to 
> "confirm" subvol is deleted? I can't come up with actual usage. Hope you 
> can help me with that.

Ok so the question is why to do it all, I think Roman answered it
quite well. As deleted subvolume can free some space, waiting until
cleaning is done is the right time to check 'df'.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] New ioctl to query deleted subvolumes
  2024-10-11 12:20       ` Roman Mamedov
@ 2024-10-12  1:45         ` Yuwei Han
  0 siblings, 0 replies; 7+ messages in thread
From: Yuwei Han @ 2024-10-12  1:45 UTC (permalink / raw)
  To: Roman Mamedov; +Cc: dsterba, linux-btrfs



在 2024/10/11 20:20, Roman Mamedov 写道:
> On Fri, 11 Oct 2024 20:06:00 +0800
> Yuwei Han <hrx@bupt.moe> wrote:
> 
>> There is a misunderstanding. What I mean is that why users need to
>> "confirm" subvol is deleted? I can't come up with actual usage. Hope you
>> can help me with that.
> 
> A backup system may delete outdated subvolumes first, then wait until the
> space has actually freed up, before copying new data; otherwise it could run
> out of disk space. Especially considering that IO from starting to copy
> without such a wait would interfere with the cleaner process IO, resulting in
> it taking a longer time to finish.
> 
> Or in my case I also wait until deleted subvolumes have been freed up *after*
> a backup, to then record how much remaining space a backup disk actually has,
> before unmounting it and putting into storage.
> 
Much thanks. It turns the async deletion operation to sync. I think it's 
reasonable.
How about introduce an universal sync command to wait all background 
cleaner process (e.g. when deleting a bunch of files, which I encountered)?

HAN Yuwei


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-10-12  1:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-09 18:03 [RFC] New ioctl to query deleted subvolumes David Sterba
2024-10-10 12:29 ` Yuwei Han
2024-10-10 17:08   ` David Sterba
2024-10-11 12:06     ` Yuwei Han
2024-10-11 12:20       ` Roman Mamedov
2024-10-12  1:45         ` Yuwei Han
2024-10-11 15:48       ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox