Skip to content

Commit fe342cf

Browse files
committed
afs: Fix checker warnings
Fix warnings raised by checker, including: (*) Warnings raised by unequal comparison for the purposes of sorting, where the endianness doesn't matter: fs/afs/addr_list.c:246:21: warning: restricted __be16 degrades to integer fs/afs/addr_list.c:246:30: warning: restricted __be16 degrades to integer fs/afs/addr_list.c:248:21: warning: restricted __be32 degrades to integer fs/afs/addr_list.c:248:49: warning: restricted __be32 degrades to integer fs/afs/addr_list.c:283:21: warning: restricted __be16 degrades to integer fs/afs/addr_list.c:283:30: warning: restricted __be16 degrades to integer (*) afs_set_cb_interest() is not actually used and can be removed. (*) afs_cell_gc_delay() should be provided with a sysctl. (*) afs_cell_destroy() needs to use rcu_access_pointer() to read cell->vl_addrs. (*) afs_init_fs_cursor() should be static. (*) struct afs_vnode::permit_cache needs to be marked __rcu. (*) afs_server_rcu() needs to use rcu_access_pointer(). (*) afs_destroy_server() should use rcu_access_pointer() on server->addresses as the server object is no longer accessible. (*) afs_find_server() casts __be16/__be32 values to int in order to directly compare them for the purpose of finding a match in a list, but is should also annotate the cast with __force to avoid checker warnings. (*) afs_check_permit() accesses vnode->permit_cache outside of the RCU readlock, though it doesn't then access the value; the extraneous access is deleted. False positives: (*) Conditional locking around the code in xdr_decode_AFSFetchStatus. This can be dealt with in a separate patch. fs/afs/fsclient.c:148:9: warning: context imbalance in 'xdr_decode_AFSFetchStatus' - different lock contexts for basic block (*) Incorrect handling of seq-retry lock context balance: fs/afs/inode.c:455:38: warning: context imbalance in 'afs_getattr' - different lock contexts for basic block fs/afs/server.c:52:17: warning: context imbalance in 'afs_find_server' - different lock contexts for basic block fs/afs/server.c:128:17: warning: context imbalance in 'afs_find_server_by_uuid' - different lock contexts for basic block Errors: (*) afs_lookup_cell_rcu() needs to break out of the seq-retry loop, not go round again if it successfully found the workstation cell. (*) Fix UUID decode in afs_deliver_cb_probe_uuid(). (*) afs_cache_permit() has a missing rcu_read_unlock() before one of the jumps to the someone_else_changed_it label. Move the unlock to after the label. (*) afs_vl_get_addrs_u() is using ntohl() rather than htonl() when encoding to XDR. (*) afs_deliver_yfsvl_get_endpoints() is using htonl() rather than ntohl() when decoding from XDR. Signed-off-by: David Howells <[email protected]>
1 parent a09acf4 commit fe342cf

File tree

11 files changed

+35
-50
lines changed

11 files changed

+35
-50
lines changed

fs/afs/addr_list.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,9 +243,9 @@ void afs_merge_fs_addr4(struct afs_addr_list *alist, __be32 xdr, u16 port)
243243
xport == a->sin6_port)
244244
return;
245245
if (xdr == a->sin6_addr.s6_addr32[3] &&
246-
xport < a->sin6_port)
246+
(u16 __force)xport < (u16 __force)a->sin6_port)
247247
break;
248-
if (xdr < a->sin6_addr.s6_addr32[3])
248+
if ((u32 __force)xdr < (u32 __force)a->sin6_addr.s6_addr32[3])
249249
break;
250250
}
251251

@@ -280,7 +280,7 @@ void afs_merge_fs_addr6(struct afs_addr_list *alist, __be32 *xdr, u16 port)
280280
xport == a->sin6_port)
281281
return;
282282
if (diff == 0 &&
283-
xport < a->sin6_port)
283+
(u16 __force)xport < (u16 __force)a->sin6_port)
284284
break;
285285
if (diff < 0)
286286
break;

fs/afs/callback.c

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -96,26 +96,6 @@ int afs_register_server_cb_interest(struct afs_vnode *vnode,
9696
return 0;
9797
}
9898

99-
/*
100-
* Set a vnode's interest on a server.
101-
*/
102-
void afs_set_cb_interest(struct afs_vnode *vnode, struct afs_cb_interest *cbi)
103-
{
104-
struct afs_cb_interest *old_cbi = NULL;
105-
106-
if (vnode->cb_interest == cbi)
107-
return;
108-
109-
write_seqlock(&vnode->cb_lock);
110-
if (vnode->cb_interest != cbi) {
111-
afs_get_cb_interest(cbi);
112-
old_cbi = vnode->cb_interest;
113-
vnode->cb_interest = cbi;
114-
}
115-
write_sequnlock(&vnode->cb_lock);
116-
afs_put_cb_interest(afs_v2net(vnode), cbi);
117-
}
118-
11999
/*
120100
* Remove an interest on a server.
121101
*/

fs/afs/cell.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
#include <keys/rxrpc-type.h>
1919
#include "internal.h"
2020

21-
unsigned __read_mostly afs_cell_gc_delay = 10;
21+
static unsigned __read_mostly afs_cell_gc_delay = 10;
2222

2323
static void afs_manage_cell(struct work_struct *);
2424

@@ -75,7 +75,7 @@ struct afs_cell *afs_lookup_cell_rcu(struct afs_net *net,
7575
cell = rcu_dereference_raw(net->ws_cell);
7676
if (cell) {
7777
afs_get_cell(cell);
78-
continue;
78+
break;
7979
}
8080
ret = -EDESTADDRREQ;
8181
continue;
@@ -411,7 +411,7 @@ static void afs_cell_destroy(struct rcu_head *rcu)
411411

412412
ASSERTCMP(atomic_read(&cell->usage), ==, 0);
413413

414-
afs_put_addrlist(cell->vl_addrs);
414+
afs_put_addrlist(rcu_access_pointer(cell->vl_addrs));
415415
key_put(cell->anonymous_key);
416416
kfree(cell);
417417

fs/afs/cmservice.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -500,9 +500,9 @@ static int afs_deliver_cb_probe_uuid(struct afs_call *call)
500500

501501
b = call->buffer;
502502
r = call->request;
503-
r->time_low = ntohl(b[0]);
504-
r->time_mid = ntohl(b[1]);
505-
r->time_hi_and_version = ntohl(b[2]);
503+
r->time_low = b[0];
504+
r->time_mid = htons(ntohl(b[1]));
505+
r->time_hi_and_version = htons(ntohl(b[2]));
506506
r->clock_seq_hi_and_reserved = ntohl(b[3]);
507507
r->clock_seq_low = ntohl(b[4]);
508508

fs/afs/inode.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ void afs_evict_inode(struct inode *inode)
544544
}
545545
#endif
546546

547-
afs_put_permits(vnode->permit_cache);
547+
afs_put_permits(rcu_access_pointer(vnode->permit_cache));
548548
_leave("");
549549
}
550550

fs/afs/internal.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ struct afs_vnode {
458458
#ifdef CONFIG_AFS_FSCACHE
459459
struct fscache_cookie *cache; /* caching cookie */
460460
#endif
461-
struct afs_permits *permit_cache; /* cache of permits so far obtained */
461+
struct afs_permits __rcu *permit_cache; /* cache of permits so far obtained */
462462
struct mutex io_lock; /* Lock for serialising I/O on this mutex */
463463
struct mutex validate_lock; /* lock for validating this vnode */
464464
spinlock_t wb_lock; /* lock for wb_keys */

fs/afs/proc.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ static int afs_proc_cells_open(struct inode *inode, struct file *file)
183183
* first item
184184
*/
185185
static void *afs_proc_cells_start(struct seq_file *m, loff_t *_pos)
186+
__acquires(rcu)
186187
{
187188
struct afs_net *net = afs_seq2net(m);
188189

@@ -204,6 +205,7 @@ static void *afs_proc_cells_next(struct seq_file *m, void *v, loff_t *pos)
204205
* clean up after reading from the cells list
205206
*/
206207
static void afs_proc_cells_stop(struct seq_file *m, void *v)
208+
__releases(rcu)
207209
{
208210
rcu_read_unlock();
209211
}
@@ -413,6 +415,7 @@ static int afs_proc_cell_volumes_open(struct inode *inode, struct file *file)
413415
* first item
414416
*/
415417
static void *afs_proc_cell_volumes_start(struct seq_file *m, loff_t *_pos)
418+
__acquires(cell->proc_lock)
416419
{
417420
struct afs_cell *cell = m->private;
418421

@@ -438,6 +441,7 @@ static void *afs_proc_cell_volumes_next(struct seq_file *p, void *v,
438441
* clean up after reading from the cells list
439442
*/
440443
static void afs_proc_cell_volumes_stop(struct seq_file *p, void *v)
444+
__releases(cell->proc_lock)
441445
{
442446
struct afs_cell *cell = p->private;
443447

@@ -500,6 +504,7 @@ static int afs_proc_cell_vlservers_open(struct inode *inode, struct file *file)
500504
* first item
501505
*/
502506
static void *afs_proc_cell_vlservers_start(struct seq_file *m, loff_t *_pos)
507+
__acquires(rcu)
503508
{
504509
struct afs_addr_list *alist;
505510
struct afs_cell *cell = m->private;
@@ -544,6 +549,7 @@ static void *afs_proc_cell_vlservers_next(struct seq_file *p, void *v,
544549
* clean up after reading from the cells list
545550
*/
546551
static void afs_proc_cell_vlservers_stop(struct seq_file *p, void *v)
552+
__releases(rcu)
547553
{
548554
rcu_read_unlock();
549555
}
@@ -580,6 +586,7 @@ static int afs_proc_servers_open(struct inode *inode, struct file *file)
580586
* first item.
581587
*/
582588
static void *afs_proc_servers_start(struct seq_file *m, loff_t *_pos)
589+
__acquires(rcu)
583590
{
584591
struct afs_net *net = afs_seq2net(m);
585592

@@ -601,6 +608,7 @@ static void *afs_proc_servers_next(struct seq_file *m, void *v, loff_t *_pos)
601608
* clean up after reading from the cells list
602609
*/
603610
static void afs_proc_servers_stop(struct seq_file *p, void *v)
611+
__releases(rcu)
604612
{
605613
rcu_read_unlock();
606614
}

fs/afs/rotate.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
/*
2222
* Initialise a filesystem server cursor for iterating over FS servers.
2323
*/
24-
void afs_init_fs_cursor(struct afs_fs_cursor *fc, struct afs_vnode *vnode)
24+
static void afs_init_fs_cursor(struct afs_fs_cursor *fc, struct afs_vnode *vnode)
2525
{
2626
memset(fc, 0, sizeof(*fc));
2727
}

fs/afs/security.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -178,18 +178,14 @@ void afs_cache_permit(struct afs_vnode *vnode, struct key *key,
178178
}
179179
}
180180

181-
if (cb_break != (vnode->cb_break + vnode->cb_interest->server->cb_s_break)) {
182-
rcu_read_unlock();
181+
if (cb_break != (vnode->cb_break + vnode->cb_interest->server->cb_s_break))
183182
goto someone_else_changed_it;
184-
}
185183

186184
/* We need a ref on any permits list we want to copy as we'll have to
187185
* drop the lock to do memory allocation.
188186
*/
189-
if (permits && !refcount_inc_not_zero(&permits->usage)) {
190-
rcu_read_unlock();
187+
if (permits && !refcount_inc_not_zero(&permits->usage))
191188
goto someone_else_changed_it;
192-
}
193189

194190
rcu_read_unlock();
195191

@@ -278,6 +274,7 @@ void afs_cache_permit(struct afs_vnode *vnode, struct key *key,
278274
/* Someone else changed the cache under us - don't recheck at this
279275
* time.
280276
*/
277+
rcu_read_unlock();
281278
return;
282279
}
283280

@@ -296,8 +293,6 @@ int afs_check_permit(struct afs_vnode *vnode, struct key *key,
296293
_enter("{%x:%u},%x",
297294
vnode->fid.vid, vnode->fid.vnode, key_serial(key));
298295

299-
permits = vnode->permit_cache;
300-
301296
/* check the permits to see if we've got one yet */
302297
if (key == vnode->volume->cell->anonymous_key) {
303298
_debug("anon");

fs/afs/server.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ struct afs_server *afs_find_server(struct afs_net *net,
5959
alist = rcu_dereference(server->addresses);
6060
for (i = alist->nr_ipv4; i < alist->nr_addrs; i++) {
6161
b = &alist->addrs[i].transport.sin6;
62-
diff = (u16)a->sin6_port - (u16)b->sin6_port;
62+
diff = ((u16 __force)a->sin6_port -
63+
(u16 __force)b->sin6_port);
6364
if (diff == 0)
6465
diff = memcmp(&a->sin6_addr,
6566
&b->sin6_addr,
@@ -79,10 +80,11 @@ struct afs_server *afs_find_server(struct afs_net *net,
7980
alist = rcu_dereference(server->addresses);
8081
for (i = 0; i < alist->nr_ipv4; i++) {
8182
b = &alist->addrs[i].transport.sin6;
82-
diff = (u16)a->sin6_port - (u16)b->sin6_port;
83+
diff = ((u16 __force)a->sin6_port -
84+
(u16 __force)b->sin6_port);
8385
if (diff == 0)
84-
diff = ((u32)a->sin6_addr.s6_addr32[3] -
85-
(u32)b->sin6_addr.s6_addr32[3]);
86+
diff = ((u32 __force)a->sin6_addr.s6_addr32[3] -
87+
(u32 __force)b->sin6_addr.s6_addr32[3]);
8688
if (diff == 0)
8789
goto found;
8890
if (diff < 0) {
@@ -381,7 +383,7 @@ static void afs_server_rcu(struct rcu_head *rcu)
381383
{
382384
struct afs_server *server = container_of(rcu, struct afs_server, rcu);
383385

384-
afs_put_addrlist(server->addresses);
386+
afs_put_addrlist(rcu_access_pointer(server->addresses));
385387
kfree(server);
386388
}
387389

@@ -390,7 +392,7 @@ static void afs_server_rcu(struct rcu_head *rcu)
390392
*/
391393
static void afs_destroy_server(struct afs_net *net, struct afs_server *server)
392394
{
393-
struct afs_addr_list *alist = server->addresses;
395+
struct afs_addr_list *alist = rcu_access_pointer(server->addresses);
394396
struct afs_addr_cursor ac = {
395397
.alist = alist,
396398
.addr = &alist->addrs[0],

0 commit comments

Comments
 (0)