From: Abbas Raza <[email protected]>
Add function to lookup a subdirectory in a directory by given name
Cc: Theodore Ts'o <[email protected]>
Cc: Alexander Viro <[email protected]>
Signed-off-by: Abbas Raza <[email protected]>
---
fs/proc/generic.c | 18 ++++++++++++++++++
include/linux/proc_fs.h | 4 ++++
2 files changed, 22 insertions(+)
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index b796da2..e4f2320 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -406,6 +406,24 @@ static const struct dentry_operations proc_dentry_operations =
};
/*
+ * Lookup if a subdirectory is present in the given directory by name
+ */
+int proc_lookup_subdir_by_name(struct proc_dir_entry *dir, const char *name)
+{
+ struct proc_dir_entry *tmp;
+ int ret = -ENOENT;
+
+ spin_lock(&proc_subdir_lock);
+ for (tmp = dir->subdir; tmp; tmp = tmp->next)
+ if (strcmp(tmp->name, name) == 0)
+ ret = 0;
+ spin_unlock(&proc_subdir_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(proc_lookup_subdir_by_name);
+
+/*
* Don't create negative dentries here, return -ENOENT by hand
* instead.
*/
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 32676b3..82acc6c 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -152,6 +152,8 @@ extern struct proc_dir_entry *proc_symlink(const char *,
extern struct proc_dir_entry *proc_mkdir(const char *,struct proc_dir_entry *);
extern struct proc_dir_entry *proc_mkdir_mode(const char *name, umode_t mode,
struct proc_dir_entry *parent);
+extern int proc_lookup_subdir_by_name(struct proc_dir_entry *,
+ const char *name);
static inline struct proc_dir_entry *proc_create(const char *name, umode_t mode,
struct proc_dir_entry *parent, const struct file_operations *proc_fops)
@@ -213,6 +215,8 @@ static inline struct proc_dir_entry *proc_mkdir(const char *name,
struct proc_dir_entry *parent) {return NULL;}
static inline struct proc_dir_entry *proc_mkdir_mode(const char *name,
umode_t mode, struct proc_dir_entry *parent) { return NULL; }
+static inline int proc_lookup_subdir_by_name(struct proc_dir_entry *,
+ const char *name) { return -ENOENT; }
static inline struct proc_dir_entry *create_proc_read_entry(const char *name,
umode_t mode, struct proc_dir_entry *base,
--
1.8.3.2
From: Abbas Raza <[email protected]>
ext4 code throws WARNINGs if a mounted card having ext4 filesystem is
ejected without unmounting first and inserted again. This happens because
ext4 procfs entries for this card doesn't get cleaned when it is ejected
this way.
Steps to reproduce this problem:
1) card having ext4 filesystem is already mounted:
root@mx6q:~# mount
/dev/mmcblk0p1 on /media/sd-mmcblk0p1 type ext4 (rw,relatime,data=ordered)
2) cd <mount point>
root@mx6q:~# cd /media/sd-mmcblk0p1/
root@mx6q:/media/sd-mmcblk0p1#
root@mx6q:/media/sd-mmcblk0p1# ls /proc/fs/ext4/
mmcblk0p1
root@mx6q:/media/sd-mmcblk0p1#
3) Remove card:
root@mx6q:/media/sd-mmcblk0p1#
[ 165.167264] mmc1: card efa2 removed
4) Proc entries for the removed card are still present:
root@mx6q:/media/sd-mmcblk0p1# ls /proc/fs/ext4/
mmcblk0p1
5) Re-insert card (mount will fail with a following warnings):
root@mx6q:/media/sd-mmcblk0p1#
[ 74.987270] mmc1: new SDHC card at address efa2
[ 74.992426] mmcblk0: mmc1:efa2 SD04G 3.69 GiB
[ 75.003689] mmcblk0: p1
[ 75.590601] ------------[ cut here ]------------
[ 75.595296] WARNING: at fs/proc/generic.c:573 proc_register+0xe8/0x138()
[ 75.602013] proc_dir_entry 'ext4/mmcblk0p1' already registered
[ 75.607882] Modules linked in: ip_tables x_tables
[ 75.662964] Backtrace:
[ 75.665500] [<80011734>] (dump_backtrace+0x0/0x10c) from [<803a2eb8>] (dump_stack+0x18/0x1c)
[ 75.674005] r6:80488400 r5:0000023d r4:ac637c60 r3:80508444
[ 75.679779] [<803a2ea0>] (dump_stack+0x0/0x1c) from [<80022908>] (warn_slowpath_common+0x54/0x6c)
[ 75.688885] [<800228b4>] (warn_slowpath_common+0x0/0x6c) from [<800229c4>] (warn_slowpath_fmt+0x38/0x40)
[ 75.698596] r8:a9968b59 r7:a816b200 r6:00000000 r5:ac1a1c80 r4:a9968b00
r3:00000009
[ 75.706615] [<8002298c>] (warn_slowpath_fmt+0x0/0x40) from [<8013b03c>] (proc_register+0xe8/0x138)
[ 75.715783] r3:ac1a1cd9 r2:80488466
[ 75.719437] [<8013af54>] (proc_register+0x0/0x138) from [<8013b264>] (proc_mkdir_mode+0x40/0x60)
[ 75.728411] r8:805bac20 r7:ac721800 r6:ac722000 r5:00000000 r4:a9968b00
[ 75.735284] [<8013b224>] (proc_mkdir_mode+0x0/0x60) from [<8013b29c>] (proc_mkdir+0x18/0x1c)
[ 75.743825] r4:0000007f
[ 75.746399] [<8013b284>] (proc_mkdir+0x0/0x1c) from [<8016f038>] (ext4_fill_super+0xe1c/0x2788)
[ 75.755193] [<8016e21c>] (ext4_fill_super+0x0/0x2788) from [<800eae6c>] (mount_bdev+0x124/0x1a0)
[ 75.764082] [<800ead48>] (mount_bdev+0x0/0x1a0) from [<8015e740>] (ext4_mount+0x1c/0x28)
[ 75.772263] [<8015e724>] (ext4_mount+0x0/0x28) from [<800eb728>] (mount_fs+0x1c/0xc0)
[ 75.780160] [<800eb70c>] (mount_fs+0x0/0xc0) from [<80102498>] (vfs_kern_mount+0x54/0xc8)
[ 75.788409] r6:00000000 r5:ac3886c0 r4:acbd4b40
[ 75.793102] [<80102444>] (vfs_kern_mount+0x0/0xc8) from [<801045e8>] (do_mount+0x71c/0x834)
[ 75.801557] r8:ac636000 r7:8051f3dc r6:805322c0 r5:ac3886c0 r4:00000020
r3:00000000
[ 75.809528] [<80103ecc>] (do_mount+0x0/0x834) from [<8010478c>] (sys_mount+0x8c/0xc0)
[ 75.817446] [<80104700>] (sys_mount+0x0/0xc0) from [<8000d860>] (ret_fast_syscall+0x0/0x30)
[ 75.825883] r7:00000015 r6:005956c0 r5:00200000 r4:00000000
[ 75.831606] ---[ end trace 041836e4555827df ]---
[ 75.839421] ------------[ cut here ]------------
[ 75.844096] WARNING: at fs/proc/generic.c:573 proc_register+0xe8/0x138()
[ 75.850810] proc_dir_entry 'jbd2/mmcblk0p1-8' already registered
...
[ 76.118095] ------------[ cut here ]------------
[ 76.122776] WARNING: at fs/sysfs/dir.c:536 sysfs_add_one+0x9c/0xbc()
[ 76.129140] sysfs: cannot create duplicate filename '/fs/ext4/mmcblk0p1'
...
[ 76.410445] ------------[ cut here ]------------
[ 76.415124] WARNING: at lib/kobject.c:196 kobject_add_internal+0x15c/0x1cc()
[ 76.422214] kobject_add_internal failed for mmcblk0p1 with -EEXIST, don't try to register things with the same name in the same directory.
...
[ 76.661850] ---[ end trace 041836e4555827e2 ]---
[ 76.666572] EXT4-fs (mmcblk0p1): mount failed
With this patch:
5) Re-insert card (mount will fail with following message):
root@mx6q:/media/sd-mmcblk0p1#
root@mx6q:/media/sd-mmcblk0p1# [ 171.392220] mmc1: new SDHC card at address efa2
[ 171.398608] mmcblk0: mmc1:efa2 SD04G 3.69 GiB
[ 171.410501] mmcblk0: p1
[ 171.990337] EXT4-fs (mmcblk0p1): could'nt mount because mount point is busy and /proc/ext4/mmcblk0p1 is already present
6) Free mount point and proc entries for the card will vanish
root@mx6q:/media/sd-mmcblk0p1# cd ..
root@mx6q:/media# ls /proc/fs/ext4/
7) Re-insert card and it will mount successfully:
root@mx6q:/media# [ 194.317263] mmc1: card efa2 removed
root@mx6q:/media# [ 199.512198] mmc1: new SDHC card at address efa2
[ 199.518254] mmcblk0: mmc1:efa2 SD04G 3.69 GiB
[ 199.527939] mmcblk0: p1
[ 200.101131] EXT4-fs (mmcblk0p1): recovery complete
[ 200.105985] EXT4-fs (mmcblk0p1): mounted filesystem with ordered data mode. Opts: (null)
root@mx6q:/media# ls /proc/fs/ext4/
mmcblk0p1
Cc: Theodore Ts'o <[email protected]>
Cc: Alexander Viro <[email protected]>
Signed-off-by: Abbas Raza <[email protected]>
---
fs/ext4/super.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index b83f75d..edb5613 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3708,8 +3708,17 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
goto failed_mount;
}
- if (ext4_proc_root)
+ if (ext4_proc_root) {
+ if (proc_lookup_subdir_by_name(ext4_proc_root,
+ sb->s_id) == 0) {
+ ext4_msg(sb, KERN_ERR, "could'nt mount because"
+ " mount point is busy and /proc/%s/%s is"
+ " already present",
+ ext4_proc_root->name, sb->s_id);
+ goto failed_mount;
+ }
sbi->s_proc = proc_mkdir(sb->s_id, ext4_proc_root);
+ }
if (sbi->s_proc)
proc_create_data("options", S_IRUGO, sbi->s_proc,
--
1.8.3.2
On Sat, Dec 21, 2013 at 03:59:16AM +0500, Abbas Raza wrote:
> /*
> + * Lookup if a subdirectory is present in the given directory by name
> + */
> +int proc_lookup_subdir_by_name(struct proc_dir_entry *dir, const char *name)
> +{
I'm not sure this is the best way to solve the problem you are trying
to get at in patch 2/2. First of all, this solution is fundamentally
racy; the moment you release proc_subdir_lock, there's always a chance
that some other process will come in and create the proc file in the
subdirectory. Sure, it's not likely in the case of the ext4 use case,
but as a generic function to be added to fs/proc/generic.c, I suspent
Al is going to NACK this.
Secondly, there is a bigger problem that this is papering over, which
is we don't have a good way of dealing with an unclean eject. If the
the system was in the middle of reading or writing to the file system
at the time of the unclean eject, there will be all sorts of warnings
and file system errors that will be logged. That's because we don't
have a way for the block device layer to let the file system know that
the block device has disappeared, and we need to revoke open file
descriptors, and accept the fact that any dirty pages in the cache
cache need to be deleted, lest it clog the system memory forever.
Given the larger problem, I have to admit it's hard for me to get
excited about trying to suppress this particular warning message. If
we are going to do this, it may be better to simply add a new
proc_mkdir_flags() interface which extends proc_mkdir_data() with a
new integer flags parameter, for which we define a new flag, which
suppresses the warning.
Regards,
- Ted