[PATCH] fs/file.c: tidy up close_files

[PATCH] fs/file.c: tidy up close_files

[Mateusz Guzik]

Improve consistency by reorganizing the code to use the same constructs
as do_close_on_exec.

Get rid of xchg of the file pointer. Since the table is about to be
freed there is no reason to NULLify the slot.

Get rid of the file NULL check. At this stage a bit set is supposed to
guarantee the slot is populated.

Signed-off-by: Mateusz Guzik <mguzik@redhat.com>
---
 fs/file.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 6c672ad..103d931 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -365,7 +365,7 @@ out:
 	return NULL;
 }
 
-static struct fdtable *close_files(struct files_struct * files)
+static struct fdtable *close_files(struct files_struct *files)
 {
 	/*
 	 * It is safe to dereference the fd table without RCU or
@@ -373,24 +373,19 @@ static struct fdtable *close_files(struct files_struct * files)
 	 * files structure.
 	 */
 	struct fdtable *fdt = rcu_dereference_raw(files->fdt);
-	int i, j = 0;
+	unsigned i;
 
-	for (;;) {
+	for (i = 0; ; i++) {
 		unsigned long set;
-		i = j * BITS_PER_LONG;
-		if (i >= fdt->max_fds)
+		unsigned fd = i * BITS_PER_LONG;
+		if (fd >= fdt->max_fds)
 			break;
-		set = fdt->open_fds[j++];
-		while (set) {
-			if (set & 1) {
-				struct file * file = xchg(&fdt->fd[i], NULL);
-				if (file) {
-					filp_close(file, files);
-					cond_resched_rcu_qs();
-				}
-			}
-			i++;
-			set >>= 1;
+		set = fdt->open_fds[i];
+		for ( ; set ; fd++, set >>= 1) {
+			if (!(set & 1))
+				continue;
+			filp_close(fdt->fd[fd], files);
+			cond_resched_rcu_qs();
 		}
 	}
 
-- 
1.8.3.1

Re: [PATCH] fs/file.c: tidy up close_files

[kbuild test robot]

Hi Mateusz,

[auto build test WARNING on v4.3-rc4 -- if it's inappropriate base, please ignore]

reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   fs/file.c:321:17: sparse: incorrect type in assignment (different address spaces)
   fs/file.c:321:17:    expected struct file **old_fds
   fs/file.c:321:17:    got struct file [noderef] <asn:4>**fd
   fs/file.c:322:17: sparse: incorrect type in assignment (different address spaces)
   fs/file.c:322:17:    expected struct file **new_fds
   fs/file.c:322:17:    got struct file [noderef] <asn:4>**fd
>> fs/file.c:387:43: sparse: incorrect type in argument 1 (different address spaces)
   fs/file.c:387:43:    expected struct file *<noident>
   fs/file.c:387:43:    got struct file [noderef] <asn:4>*<noident>
   fs/file.c:446:28: sparse: incorrect type in initializer (different address spaces)
   fs/file.c:446:28:    expected struct fdtable [noderef] <asn:4>*fdt
   fs/file.c:446:28:    got struct fdtable [toplevel] *<noident>
   fs/file.c:606:14: sparse: incorrect type in assignment (different address spaces)
   fs/file.c:606:14:    expected struct file *file
   fs/file.c:606:14:    got struct file [noderef] <asn:4>*<noident>
   fs/file.c:641:30: sparse: incorrect type in assignment (different address spaces)
   fs/file.c:641:30:    expected struct file *file
   fs/file.c:641:30:    got struct file [noderef] <asn:4>*<noident>
   fs/file.c:802:16: sparse: incorrect type in assignment (different address spaces)
   fs/file.c:802:16:    expected struct file *tofree
   fs/file.c:802:16:    got struct file [noderef] <asn:4>*<noident>

vim +387 fs/file.c

   371		 * It is safe to dereference the fd table without RCU or
   372		 * ->file_lock because this is the last reference to the
   373		 * files structure.
   374		 */
   375		struct fdtable *fdt = rcu_dereference_raw(files->fdt);
   376		unsigned i;
   377	
   378		for (i = 0; ; i++) {
   379			unsigned long set;
   380			unsigned fd = i * BITS_PER_LONG;
   381			if (fd >= fdt->max_fds)
   382				break;
   383			set = fdt->open_fds[i];
   384			for ( ; set ; fd++, set >>= 1) {
   385				if (!(set & 1))
   386					continue;
 > 387				filp_close(fdt->fd[fd], files);
   388				cond_resched_rcu_qs();
   389			}
   390		}
   391	
   392		return fdt;
   393	}
   394	
   395	struct files_struct *get_files_struct(struct task_struct *task)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation