Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

locking in fs/9p ->readdir()

... is really excessive. First of all, ->readdir() is serialized by
file->f_path.dentry->d_inode->i_mutex; playing with file->f_path.dentry->d_lock
is not buying you anything. Moreover, rdir->mutex is pointless for exactly
the same reason - you'll never see contention on it.

While we are at it, there's no point in having rdir->buf a pointer -
you have it point just past the end of rdir, so it might as well be a flex
array (and no, it's not a gccism).

Absolutely untested patch follows:

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Eric Van Hensbergen <ericvh@gmail.com>

authored by

Al Viro and committed by
Eric Van Hensbergen
7ffdea7e 836dc9e3

+23 -69
+23 -69
fs/9p/vfs_dir.c
··· 52 52 */ 53 53 54 54 struct p9_rdir { 55 - struct mutex mutex; 56 55 int head; 57 56 int tail; 58 - uint8_t *buf; 57 + uint8_t buf[]; 59 58 }; 60 59 61 60 /** ··· 92 93 * 93 94 */ 94 95 95 - static int v9fs_alloc_rdir_buf(struct file *filp, int buflen) 96 + static struct p9_rdir *v9fs_alloc_rdir_buf(struct file *filp, int buflen) 96 97 { 97 - struct p9_rdir *rdir; 98 - struct p9_fid *fid; 99 - int err = 0; 100 - 101 - fid = filp->private_data; 102 - if (!fid->rdir) { 103 - rdir = kmalloc(sizeof(struct p9_rdir) + buflen, GFP_KERNEL); 104 - 105 - if (rdir == NULL) { 106 - err = -ENOMEM; 107 - goto exit; 108 - } 109 - spin_lock(&filp->f_dentry->d_lock); 110 - if (!fid->rdir) { 111 - rdir->buf = (uint8_t *)rdir + sizeof(struct p9_rdir); 112 - mutex_init(&rdir->mutex); 113 - rdir->head = rdir->tail = 0; 114 - fid->rdir = (void *) rdir; 115 - rdir = NULL; 116 - } 117 - spin_unlock(&filp->f_dentry->d_lock); 118 - kfree(rdir); 119 - } 120 - exit: 121 - return err; 98 + struct p9_fid *fid = filp->private_data; 99 + if (!fid->rdir) 100 + fid->rdir = kzalloc(sizeof(struct p9_rdir) + buflen, GFP_KERNEL); 101 + return fid->rdir; 122 102 } 123 103 124 104 /** ··· 123 145 124 146 buflen = fid->clnt->msize - P9_IOHDRSZ; 125 147 126 - err = v9fs_alloc_rdir_buf(filp, buflen); 127 - if (err) 128 - goto exit; 129 - rdir = (struct p9_rdir *) fid->rdir; 148 + rdir = v9fs_alloc_rdir_buf(filp, buflen); 149 + if (!rdir) 150 + return -ENOMEM; 130 151 131 - err = mutex_lock_interruptible(&rdir->mutex); 132 - if (err) 133 - return err; 134 - while (err == 0) { 152 + while (1) { 135 153 if (rdir->tail == rdir->head) { 136 154 err = v9fs_file_readn(filp, rdir->buf, NULL, 137 155 buflen, filp->f_pos); 138 156 if (err <= 0) 139 - goto unlock_and_exit; 157 + return err; 140 158 141 159 rdir->head = 0; 142 160 rdir->tail = err; ··· 143 169 rdir->tail - rdir->head, &st); 144 170 if (err) { 145 171 p9_debug(P9_DEBUG_VFS, "returned %d\n", err); 146 - err = -EIO; 147 172 p9stat_free(&st); 148 - goto unlock_and_exit; 173 + return -EIO; 149 174 } 150 175 reclen = st.size+2; 151 176 ··· 153 180 154 181 p9stat_free(&st); 155 182 156 - if (over) { 157 - err = 0; 158 - goto unlock_and_exit; 159 - } 183 + if (over) 184 + return 0; 185 + 160 186 rdir->head += reclen; 161 187 filp->f_pos += reclen; 162 188 } 163 189 } 164 - 165 - unlock_and_exit: 166 - mutex_unlock(&rdir->mutex); 167 - exit: 168 - return err; 169 190 } 170 191 171 192 /** ··· 185 218 186 219 buflen = fid->clnt->msize - P9_READDIRHDRSZ; 187 220 188 - err = v9fs_alloc_rdir_buf(filp, buflen); 189 - if (err) 190 - goto exit; 191 - rdir = (struct p9_rdir *) fid->rdir; 221 + rdir = v9fs_alloc_rdir_buf(filp, buflen); 222 + if (!rdir) 223 + return -ENOMEM; 192 224 193 - err = mutex_lock_interruptible(&rdir->mutex); 194 - if (err) 195 - return err; 196 - 197 - while (err == 0) { 225 + while (1) { 198 226 if (rdir->tail == rdir->head) { 199 227 err = p9_client_readdir(fid, rdir->buf, buflen, 200 228 filp->f_pos); 201 229 if (err <= 0) 202 - goto unlock_and_exit; 230 + return err; 203 231 204 232 rdir->head = 0; 205 233 rdir->tail = err; ··· 207 245 &curdirent); 208 246 if (err < 0) { 209 247 p9_debug(P9_DEBUG_VFS, "returned %d\n", err); 210 - err = -EIO; 211 - goto unlock_and_exit; 248 + return -EIO; 212 249 } 213 250 214 251 /* d_off in dirent structure tracks the offset into ··· 222 261 curdirent.d_type); 223 262 oldoffset = curdirent.d_off; 224 263 225 - if (over) { 226 - err = 0; 227 - goto unlock_and_exit; 228 - } 264 + if (over) 265 + return 0; 229 266 230 267 filp->f_pos = curdirent.d_off; 231 268 rdir->head += err; 232 269 } 233 270 } 234 - 235 - unlock_and_exit: 236 - mutex_unlock(&rdir->mutex); 237 - exit: 238 - return err; 239 271 } 240 272 241 273