Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1587639 > unrolled thread

[PATCH 00/14] AFS: Fixes and cleanups

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/14] AFS: Fixes and cleanups David Howells <dhowells@redhat.com> - 2017-02-24 14:20 +0100
    [PATCH 11/14] afs: inode: Replace rcu_assign_pointer() with  RCU_INIT_POINTER() David Howells <dhowells@redhat.com> - 2017-02-24 14:20 +0100
    [PATCH 08/14] afs: Handle a short write to an AFS page David Howells <dhowells@redhat.com> - 2017-02-24 14:30 +0100
    [PATCH 04/14] afs: Adjust mode bits processing David Howells <dhowells@redhat.com> - 2017-02-24 14:30 +0100
    [PATCH 02/14] afs: Fix page overput in afs_fill_page() David Howells <dhowells@redhat.com> - 2017-02-24 14:30 +0100
    [PATCH 09/14] afs: Flush outstanding writes when an fd is closed David Howells <dhowells@redhat.com> - 2017-02-24 14:30 +0100
    [PATCH 14/14] afs: Prevent callback expiry timer overflow David Howells <dhowells@redhat.com> - 2017-02-24 14:30 +0100
    [PATCH 03/14] afs: Populate group ID from vnode status David Howells <dhowells@redhat.com> - 2017-02-24 14:30 +0100

#1587639 — [PATCH 00/14] AFS: Fixes and cleanups

Hi Al,

Here are some fixes to the AFS filesystem, including:

 (1) Refcount errors.

 (2) Showing the GID from the server in stat.

 (3) Fix handling of AFS mode bits (which don't work quite the same way as
     Linux mode bits).

 (4) Handle a callback where the FID array and the callback array arguments
     are not populated with the same number of entries (the callback array
     may be empty).

 (5) Better handle FS.FetchData{,64} returning more or less data than
     requested (usually because the EOF pointer shifted).

 (6) Make afs_write_end() handle a short write to a page (ie. EFAULT).

 (7) Flush outstanding writes on closing a file to be consistent with NFS,
     CIFS and other AFS clients.

 (8) Mountpoints (which appear as special symlinks with mode 0644) can be
     differentiated from real symlinks (with mode 0777) can be
     distinguished on mode alone and so we shouldn't try parsing the
     symlink.

 (9) RCU initialisation fixes.

(10) Internal time field 64-bit-isation.

The patches can be found here also:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=afs

Tagged thusly:

	git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
	afs-20170224

David
---
Andreea-Cristina Bernat (2):
      afs: inode: Replace rcu_assign_pointer() with RCU_INIT_POINTER()
      afs: security: Replace rcu_assign_pointer() with RCU_INIT_POINTER()

David Howells (7):
      afs: Fix missing put_page()
      afs: Fix page overput in afs_fill_page()
      afs: Handle better the server returning excess or short data
      afs: Kill struct afs_read::pg_offset
      afs: Handle a short write to an AFS page
      afs: Flush outstanding writes when an fd is closed
      afs: Distinguish mountpoints from symlinks by file mode alone

Marc Dionne (3):
      afs: Populate group ID from vnode status
      afs: Adjust mode bits processing
      afs: Deal with an empty callback array

Tina Ruchandani (2):
      afs: Migrate vlocation fields to 64-bit
      afs: Prevent callback expiry timer overflow


 fs/afs/callback.c  |    7 ++++---
 fs/afs/cmservice.c |   11 +++++------
 fs/afs/file.c      |    8 ++++++--
 fs/afs/fsclient.c  |   53 +++++++++++++++++++++++++++++++++++++---------------
 fs/afs/inode.c     |   40 +++++++++++++++++++++------------------
 fs/afs/internal.h  |   21 ++++++++++++---------
 fs/afs/mntpt.c     |   53 ----------------------------------------------------
 fs/afs/security.c  |    9 +++++++--
 fs/afs/server.c    |    6 +++---
 fs/afs/vlocation.c |   16 +++++++++-------
 fs/afs/write.c     |   44 ++++++++++++++++++++++++++++++++++---------
 11 files changed, 140 insertions(+), 128 deletions(-)

[toc] | [next] | [standalone]

#1587640 — [PATCH 11/14] afs: inode: Replace rcu_assign_pointer() with RCU_INIT_POINTER()

From: Andreea-Cristina Bernat <bernat.ada@gmail.com>

The use of "rcu_assign_pointer()" is NULLing out the pointer.
According to RCU_INIT_POINTER()'s block comment:
"1.   This use of RCU_INIT_POINTER() is NULLing out the pointer"
it is better to use it instead of rcu_assign_pointer() because it has a
smaller overhead.

The following Coccinelle semantic patch was used:
@@
@@

- rcu_assign_pointer
+ RCU_INIT_POINTER
  (..., NULL)

Signed-off-by: Andreea-Cristina Bernat <bernat.ada@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 8f4e70566e44..5707272adf18 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -447,7 +447,7 @@ void afs_evict_inode(struct inode *inode)
 
 	mutex_lock(&vnode->permits_lock);
 	permits = vnode->permits;
-	rcu_assign_pointer(vnode->permits, NULL);
+	RCU_INIT_POINTER(vnode->permits, NULL);
 	mutex_unlock(&vnode->permits_lock);
 	if (permits)
 		call_rcu(&permits->rcu, afs_zap_permits);

[toc] | [prev] | [next] | [standalone]

#1587653 — [PATCH 08/14] afs: Handle a short write to an AFS page

Handle the situation where afs_write_begin() is told to expect that a
full-page write will be made, but this doesn't happen (EFAULT, CTRL-C,
etc.), and so afs_write_end() sees a partial write took place.  Currently,
no attempt is to deal with the discrepency.

Fix this by loading the gap from the server.

Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/fsclient.c |    4 +++-
 fs/afs/internal.h |    2 +-
 fs/afs/write.c    |   28 +++++++++++++++++++---------
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/fs/afs/fsclient.c b/fs/afs/fsclient.c
index bf8904a1a58f..6f917dd1238c 100644
--- a/fs/afs/fsclient.c
+++ b/fs/afs/fsclient.c
@@ -393,8 +393,10 @@ static int afs_deliver_fs_fetch_data(struct afs_call *call)
 			if (req->remain > 0) {
 				call->offset = 0;
 				req->index++;
-				if (req->index >= req->nr_pages)
+				if (req->index >= req->nr_pages) {
+					call->unmarshall = 4;
 					goto begin_discard;
+				}
 				goto begin_page;
 			}
 		}
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 4213aa35deed..4ffd50de9386 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -130,7 +130,7 @@ struct afs_call_type {
  */
 struct afs_read {
 	loff_t			pos;		/* Where to start reading */
-	loff_t			len;		/* How much to read */
+	loff_t			len;		/* How much we're asking for */
 	loff_t			actual_len;	/* How much we're actually getting */
 	atomic_t		usage;
 	unsigned int		remain;		/* Amount remaining */
diff --git a/fs/afs/write.c b/fs/afs/write.c
index 3ac52f6a96ff..ea66890fc188 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -84,10 +84,9 @@ void afs_put_writeback(struct afs_writeback *wb)
  * partly or wholly fill a page that's under preparation for writing
  */
 static int afs_fill_page(struct afs_vnode *vnode, struct key *key,
-			 loff_t pos, struct page *page)
+			 loff_t pos, unsigned int len, struct page *page)
 {
 	struct afs_read *req;
-	loff_t i_size;
 	int ret;
 
 	_enter(",,%llu", (unsigned long long)pos);
@@ -99,16 +98,11 @@ static int afs_fill_page(struct afs_vnode *vnode, struct key *key,
 
 	atomic_set(&req->usage, 1);
 	req->pos = pos;
+	req->len = len;
 	req->nr_pages = 1;
 	req->pages[0] = page;
 	get_page(page);
 
-	i_size = i_size_read(&vnode->vfs_inode);
-	if (pos + PAGE_SIZE > i_size)
-		req->len = i_size - pos;
-	else
-		req->len = PAGE_SIZE;
-
 	ret = afs_vnode_fetch_data(vnode, key, req);
 	afs_put_read(req);
 	if (ret < 0) {
@@ -164,7 +158,7 @@ int afs_write_begin(struct file *file, struct address_space *mapping,
 	/* page won't leak in error case: it eventually gets cleaned off LRU */
 
 	if (!PageUptodate(page) && len != PAGE_SIZE) {
-		ret = afs_fill_page(vnode, key, index << PAGE_SHIFT, page);
+		ret = afs_fill_page(vnode, key, pos & PAGE_MASK, PAGE_SIZE, page);
 		if (ret < 0) {
 			kfree(candidate);
 			_leave(" = %d [prep]", ret);
@@ -258,7 +252,9 @@ int afs_write_end(struct file *file, struct address_space *mapping,
 		  struct page *page, void *fsdata)
 {
 	struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
+	struct key *key = file->private_data;
 	loff_t i_size, maybe_i_size;
+	int ret;
 
 	_enter("{%x:%u},{%lx}",
 	       vnode->fid.vid, vnode->fid.vnode, page->index);
@@ -274,6 +270,20 @@ int afs_write_end(struct file *file, struct address_space *mapping,
 		spin_unlock(&vnode->writeback_lock);
 	}
 
+	if (!PageUptodate(page)) {
+		if (copied < len) {
+			/* Try and load any missing data from the server.  The
+			 * unmarshalling routine will take care of clearing any
+			 * bits that are beyond the EOF.
+			 */
+			ret = afs_fill_page(vnode, key, pos + copied,
+					    len - copied, page);
+			if (ret < 0)
+				return ret;
+		}
+		SetPageUptodate(page);
+	}
+
 	set_page_dirty(page);
 	if (PageDirty(page))
 		_debug("dirtied");

[toc] | [prev] | [next] | [standalone]

#1587654 — [PATCH 04/14] afs: Adjust mode bits processing

From: Marc Dionne <marc.dionne@auristor.com>

Mode bits for an afs file should not be enforced in the usual
way.

For files, the absence of user bits can restrict file access
with respect to what is granted by the server.

These bits apply regardless of the owner or the current uid; the
rest of the mode bits (group, other) are ignored.

Signed-off-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/security.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/afs/security.c b/fs/afs/security.c
index 8d010422dc89..bfa9d3428383 100644
--- a/fs/afs/security.c
+++ b/fs/afs/security.c
@@ -340,17 +340,22 @@ int afs_permission(struct inode *inode, int mask)
 	} else {
 		if (!(access & AFS_ACE_LOOKUP))
 			goto permission_denied;
+		if ((mask & MAY_EXEC) && !(inode->i_mode & S_IXUSR))
+			goto permission_denied;
 		if (mask & (MAY_EXEC | MAY_READ)) {
 			if (!(access & AFS_ACE_READ))
 				goto permission_denied;
+			if (!(inode->i_mode & S_IRUSR))
+				goto permission_denied;
 		} else if (mask & MAY_WRITE) {
 			if (!(access & AFS_ACE_WRITE))
 				goto permission_denied;
+			if (!(inode->i_mode & S_IWUSR))
+				goto permission_denied;
 		}
 	}
 
 	key_put(key);
-	ret = generic_permission(inode, mask);
 	_leave(" = %d", ret);
 	return ret;
 

[toc] | [prev] | [next] | [standalone]

#1587657 — [PATCH 02/14] afs: Fix page overput in afs_fill_page()

afs_fill_page() loads the page it wants to fill into the afs_read request
without incrementing its refcount - but then calls afs_put_read() to clean
up afterwards, which then releases a ref on the page.

Fix this by getting a ref on the page before calling
afs_vnode_fetch_data().

This causes sync after a write to hang in afs_writepages_region() because
find_get_pages_tag() gets confused and doesn't return.

Fixes: 196ee9cd2d04 ("afs: Make afs_fs_fetch_data() take a list of pages")
Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Marc Dionne <marc.dionne@auristor.com>
---

 fs/afs/write.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/afs/write.c b/fs/afs/write.c
index e919e64cd4e0..3ac52f6a96ff 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -101,6 +101,7 @@ static int afs_fill_page(struct afs_vnode *vnode, struct key *key,
 	req->pos = pos;
 	req->nr_pages = 1;
 	req->pages[0] = page;
+	get_page(page);
 
 	i_size = i_size_read(&vnode->vfs_inode);
 	if (pos + PAGE_SIZE > i_size)

[toc] | [prev] | [next] | [standalone]

#1587658 — [PATCH 09/14] afs: Flush outstanding writes when an fd is closed

Flush outstanding writes in afs when an fd is closed.  This is what NFS and
CIFS do.

Reported-by: Marc Dionne <marc.c.dionne@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/file.c     |    1 +
 fs/afs/internal.h |    1 +
 fs/afs/write.c    |   14 ++++++++++++++
 3 files changed, 16 insertions(+)

diff --git a/fs/afs/file.c b/fs/afs/file.c
index a38e1c30d110..b5829443ff69 100644
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -30,6 +30,7 @@ static int afs_readpages(struct file *filp, struct address_space *mapping,
 
 const struct file_operations afs_file_operations = {
 	.open		= afs_open,
+	.flush		= afs_flush,
 	.release	= afs_release,
 	.llseek		= generic_file_llseek,
 	.read_iter	= generic_file_read_iter,
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 4ffd50de9386..f57fd8e9728d 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -720,6 +720,7 @@ extern int afs_writepages(struct address_space *, struct writeback_control *);
 extern void afs_pages_written_back(struct afs_vnode *, struct afs_call *);
 extern ssize_t afs_file_write(struct kiocb *, struct iov_iter *);
 extern int afs_writeback_all(struct afs_vnode *);
+extern int afs_flush(struct file *, fl_owner_t);
 extern int afs_fsync(struct file *, loff_t, loff_t, int);
 
 
diff --git a/fs/afs/write.c b/fs/afs/write.c
index ea66890fc188..f1450ea09406 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -758,6 +758,20 @@ int afs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 }
 
 /*
+ * Flush out all outstanding writes on a file opened for writing when it is
+ * closed.
+ */
+int afs_flush(struct file *file, fl_owner_t id)
+{
+	_enter("");
+
+	if ((file->f_mode & FMODE_WRITE) == 0)
+		return 0;
+
+	return vfs_fsync(file, 0);
+}
+
+/*
  * notification that a previously read-only page is about to become writable
  * - if it returns an error, the caller will deliver a bus error signal
  */

[toc] | [prev] | [next] | [standalone]

#1587662 — [PATCH 14/14] afs: Prevent callback expiry timer overflow

From: Tina Ruchandani <ruchandani.tina@gmail.com>

get_seconds() returns real wall-clock seconds. On 32-bit systems
this value will overflow in year 2038 and beyond. This patch changes
afs_vnode record to use ktime_get_real_seconds() instead, for the
fields cb_expires and cb_expires_at.

Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/fsclient.c |    2 +-
 fs/afs/inode.c    |    7 ++++---
 fs/afs/internal.h |    4 ++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/afs/fsclient.c b/fs/afs/fsclient.c
index 6f917dd1238c..c05452a09398 100644
--- a/fs/afs/fsclient.c
+++ b/fs/afs/fsclient.c
@@ -145,7 +145,7 @@ static void xdr_decode_AFSCallBack(const __be32 **_bp, struct afs_vnode *vnode)
 	vnode->cb_version	= ntohl(*bp++);
 	vnode->cb_expiry	= ntohl(*bp++);
 	vnode->cb_type		= ntohl(*bp++);
-	vnode->cb_expires	= vnode->cb_expiry + get_seconds();
+	vnode->cb_expires	= vnode->cb_expiry + ktime_get_real_seconds();
 	*_bp = bp;
 }
 
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 5707272adf18..1614998886f3 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -246,12 +246,13 @@ struct inode *afs_iget(struct super_block *sb, struct key *key,
 			vnode->cb_version = 0;
 			vnode->cb_expiry = 0;
 			vnode->cb_type = 0;
-			vnode->cb_expires = get_seconds();
+			vnode->cb_expires = ktime_get_real_seconds();
 		} else {
 			vnode->cb_version = cb->version;
 			vnode->cb_expiry = cb->expiry;
 			vnode->cb_type = cb->type;
-			vnode->cb_expires = vnode->cb_expiry + get_seconds();
+			vnode->cb_expires = vnode->cb_expiry +
+				ktime_get_real_seconds();
 		}
 	}
 
@@ -324,7 +325,7 @@ int afs_validate(struct afs_vnode *vnode, struct key *key)
 	    !test_bit(AFS_VNODE_CB_BROKEN, &vnode->flags) &&
 	    !test_bit(AFS_VNODE_MODIFIED, &vnode->flags) &&
 	    !test_bit(AFS_VNODE_ZAP_DATA, &vnode->flags)) {
-		if (vnode->cb_expires < get_seconds() + 10) {
+		if (vnode->cb_expires < ktime_get_real_seconds() + 10) {
 			_debug("callback expired");
 			set_bit(AFS_VNODE_CB_BROKEN, &vnode->flags);
 		} else {
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 6a7fa9975cee..c956ccbe50b0 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -377,8 +377,8 @@ struct afs_vnode {
 	struct rb_node		server_rb;	/* link in server->fs_vnodes */
 	struct rb_node		cb_promise;	/* link in server->cb_promises */
 	struct work_struct	cb_broken_work;	/* work to be done on callback break */
-	time_t			cb_expires;	/* time at which callback expires */
-	time_t			cb_expires_at;	/* time used to order cb_promise */
+	time64_t		cb_expires;	/* time at which callback expires */
+	time64_t		cb_expires_at;	/* time used to order cb_promise */
 	unsigned		cb_version;	/* callback version */
 	unsigned		cb_expiry;	/* callback expiry time */
 	afs_callback_type_t	cb_type;	/* type of callback */

[toc] | [prev] | [next] | [standalone]

#1587667 — [PATCH 03/14] afs: Populate group ID from vnode status

From: Marc Dionne <marc.dionne@auristor.com>

The group was hard coded to GLOBAL_ROOT_GID; use the group
ID that was received from the server.

Signed-off-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 86cc7264c21c..df9fa4ddcf3a 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -70,7 +70,7 @@ static int afs_inode_map_status(struct afs_vnode *vnode, struct key *key)
 
 	set_nlink(inode, vnode->status.nlink);
 	inode->i_uid		= vnode->status.owner;
-	inode->i_gid		= GLOBAL_ROOT_GID;
+	inode->i_gid            = vnode->status.group;
 	inode->i_size		= vnode->status.size;
 	inode->i_ctime.tv_sec	= vnode->status.mtime_server;
 	inode->i_ctime.tv_nsec	= 0;

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web