[PATCH] ubifs: return ENOSPC if running out of inode number

[PATCH] ubifs: return ENOSPC if running out of inode number

[Sheng Yong]

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 fs/ubifs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 528369f..00517dc 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -164,7 +164,7 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir,
 			ubifs_err(c, "out of inode numbers");
 			make_bad_inode(inode);
 			iput(inode);
-			return ERR_PTR(-EINVAL);
+			return ERR_PTR(-ENOSPC);
 		}
 		ubifs_warn(c, "running out of inode numbers (current %lu, max %u)",
 			   (unsigned long)c->highest_inum, INUM_WATERMARK);
-- 
2.10.1

Re: [PATCH] ubifs: return ENOSPC if running out of inode number

[Richard Weinberger]

Sheng Yong,

Am 07.02.2017 um 08:28 schrieb Sheng Yong:
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> ---
>  fs/ubifs/dir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 528369f..00517dc 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -164,7 +164,7 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir,
>  			ubifs_err(c, "out of inode numbers");
>  			make_bad_inode(inode);
>  			iput(inode);
> -			return ERR_PTR(-EINVAL);
> +			return ERR_PTR(-ENOSPC);

Can you please explain *why* this has to be changed to -ENOSPC?

Thanks,
//richard

Re: [PATCH] ubifs: return ENOSPC if running out of inode number

[Sheng Yong]

On 2/7/2017 3:57 PM, Richard Weinberger wrote:
> Sheng Yong,
> 
> Am 07.02.2017 um 08:28 schrieb Sheng Yong:
>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>> ---
>>  fs/ubifs/dir.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
>> index 528369f..00517dc 100644
>> --- a/fs/ubifs/dir.c
>> +++ b/fs/ubifs/dir.c
>> @@ -164,7 +164,7 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, struct inode *dir,
>>  			ubifs_err(c, "out of inode numbers");
>>  			make_bad_inode(inode);
>>  			iput(inode);
>> -			return ERR_PTR(-EINVAL);
>> +			return ERR_PTR(-ENOSPC);
> 
> Can you please explain *why* this has to be changed to -ENOSPC?
Hi, Richard,

This is a trivial change. I think if there is not enough inode number available,
it means the filesystem has no room for the new file. So ENOSPC may be appropriate,
and some others filesystems returns ENOSPC in such scenario :)

thanks,
Sheng
> 
> Thanks,
> //richard
> 
> .
> 

Re: [PATCH] ubifs: return ENOSPC if running out of inode number

[Richard Weinberger]

Sheng Yong,

Am 07.02.2017 um 09:19 schrieb Sheng Yong:
>> Can you please explain *why* this has to be changed to -ENOSPC?
> Hi, Richard,
> 
> This is a trivial change. I think if there is not enough inode number available,
> it means the filesystem has no room for the new file. So ENOSPC may be appropriate,
> and some others filesystems returns ENOSPC in such scenario :)

It is less trivial than you might think.
UBIFS cannot reuse inode numbers, as soon you reach INUM_WATERMARK
the filesystem is more or less dead. -ENOSPC indicates that the user
can produce free space by deleting files, which will *not* help.
That's why we use -EINVAL in terms of "we are in bad state". :)

Unless you can show me an actual breakage because of -EINVAL instead
of -ENOSPC I'd keep it as-is.
Did you hit that code path? It is more likely to wear out the flash
long before you hit that limit.

Thanks,
//richard

Re: [PATCH] ubifs: return ENOSPC if running out of inode number

[Sheng Yong]

On 2/7/2017 5:25 PM, Richard Weinberger wrote:
> Sheng Yong,
> 
> Am 07.02.2017 um 09:19 schrieb Sheng Yong:
>>> Can you please explain *why* this has to be changed to -ENOSPC?
>> Hi, Richard,
>>
>> This is a trivial change. I think if there is not enough inode number available,
>> it means the filesystem has no room for the new file. So ENOSPC may be appropriate,
>> and some others filesystems returns ENOSPC in such scenario :)
> 
> It is less trivial than you might think.
> UBIFS cannot reuse inode numbers, as soon you reach INUM_WATERMARK
> the filesystem is more or less dead. -ENOSPC indicates that the user
> can produce free space by deleting files, which will *not* help.
> That's why we use -EINVAL in terms of "we are in bad state". :)
Right. This makes sense :)
> 
> Unless you can show me an actual breakage because of -EINVAL instead
> of -ENOSPC I'd keep it as-is.
> Did you hit that code path? It is more likely to wear out the flash
> long before you hit that limit.
No I didn't hit this. The watermark is large enough.

thanks,
Sheng
> 
> Thanks,
> //richard
> 
> .
>