forked from torvalds/linux
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This fixes up most of the things pointed out by akpm and Pavel Machek with comments below indicating why some things have been left: Andrew Morton wrote: > >> +static struct nodeinfo *nodeid2nodeinfo(int nodeid, gfp_t alloc) >> +{ >> + struct nodeinfo *ni; >> + int r; >> + int n; >> + >> + down_read(&nodeinfo_lock); > > Given that this function can sleep, I wonder if `alloc' is useful. > > I see lots of callers passing in a literal "0" for `alloc'. That's in fact > a secret (GFP_ATOMIC & ~__GFP_HIGH). I doubt if that's what you really > meant. Particularly as the code could at least have used __GFP_WAIT (aka > GFP_NOIO) which is much, much more reliable than "0". In fact "0" is the > least reliable mode possible. > > IOW, this is all bollixed up. When 0 is passed into nodeid2nodeinfo the function does not try to allocate a new structure at all. it's an indication that the caller only wants the nodeinfo struct for that nodeid if there actually is one in existance. I've tidied the function itself so it's more obvious, (and tidier!) >> +/* Data received from remote end */ >> +static int receive_from_sock(void) >> +{ >> + int ret = 0; >> + struct msghdr msg; >> + struct kvec iov[2]; >> + unsigned len; >> + int r; >> + struct sctp_sndrcvinfo *sinfo; >> + struct cmsghdr *cmsg; >> + struct nodeinfo *ni; >> + >> + /* These two are marginally too big for stack allocation, but this >> + * function is (currently) only called by dlm_recvd so static should be >> + * OK. >> + */ >> + static struct sockaddr_storage msgname; >> + static char incmsg[CMSG_SPACE(sizeof(struct sctp_sndrcvinfo))]; > > whoa. This is globally singly-threaded code?? Yes. it is only ever run in the context of dlm_recvd. >> >> +static void initiate_association(int nodeid) >> +{ >> + struct sockaddr_storage rem_addr; >> + static char outcmsg[CMSG_SPACE(sizeof(struct sctp_sndrcvinfo))]; > > Another static buffer to worry about. Globally singly-threaded code? Yes. Only ever called by dlm_sendd. >> + >> +/* Send a message */ >> +static int send_to_sock(struct nodeinfo *ni) >> +{ >> + int ret = 0; >> + struct writequeue_entry *e; >> + int len, offset; >> + struct msghdr outmsg; >> + static char outcmsg[CMSG_SPACE(sizeof(struct sctp_sndrcvinfo))]; > > Singly-threaded? Yep. >> >> +static void dealloc_nodeinfo(void) >> +{ >> + int i; >> + >> + for (i=1; i<=max_nodeid; i++) { >> + struct nodeinfo *ni = nodeid2nodeinfo(i, 0); >> + if (ni) { >> + idr_remove(&nodeinfo_idr, i); > > Didn't that need locking? Not. it's only ever called at DLM shutdown after all the other threads have been stopped. >> >> +static int write_list_empty(void) >> +{ >> + int status; >> + >> + spin_lock_bh(&write_nodes_lock); >> + status = list_empty(&write_nodes); >> + spin_unlock_bh(&write_nodes_lock); >> + >> + return status; >> +} > > This function's return value is meaningless. As soon as the lock gets > dropped, the return value can get out of sync with reality. > > Looking at the caller, this _might_ happen to be OK, but it's a nasty and > dangerous thing. Really the locking should be moved into the caller. It's just an optimisation to allow the caller to schedule if there is no work to do. if something arrives immediately afterwards then it will get picked up when the process re-awakes (and it will be woken by that arrival). The 'accepting' atomic has gone completely. as Andrew pointed out it didn't really achieve much anyway. I suspect it was a plaster over some other startup or shutdown bug to be honest. Signed-off-by: Patrick Caulfield <[email protected]> Signed-off-by: Steven Whitehouse <[email protected]> Cc: Andrew Morton <[email protected]> Cc: Pavel Machek <[email protected]>
- v4.15
- v4.15-rc9
- v4.15-rc8
- v4.15-rc7
- v4.15-rc6
- v4.15-rc5
- v4.15-rc4
- v4.15-rc3
- v4.15-rc2
- v4.15-rc1
- v4.14
- v4.14-rc8
- v4.14-rc7
- v4.14-rc6
- v4.14-rc5
- v4.14-rc4
- v4.14-rc3
- v4.14-rc2
- v4.14-rc1
- v4.13
- v4.13-rc7
- v4.13-rc6
- v4.13-rc5
- v4.13-rc4
- v4.13-rc3
- v4.13-rc2
- v4.13-rc1
- v4.12
- v4.12-rc7
- v4.12-rc6
- v4.12-rc5
- v4.12-rc4
- v4.12-rc3
- v4.12-rc2
- v4.12-rc1
- v4.11
- v4.11-rc8
- v4.11-rc7
- v4.11-rc6
- v4.11-rc5
- v4.11-rc4
- v4.11-rc3
- v4.11-rc2
- v4.11-rc1
- v4.10
- v4.10-rc8
- v4.10-rc7
- v4.10-rc6
- v4.10-rc5
- v4.10-rc4
- v4.10-rc3
- v4.10-rc2
- v4.10-rc1
- v4.9
- v4.9-rc8
- v4.9-rc7
- v4.9-rc6
- v4.9-rc5
- v4.9-rc4
- v4.9-rc3
- v4.9-rc2
- v4.9-rc1
- v4.8
- v4.8-rc8
- v4.8-rc7
- v4.8-rc6
- v4.8-rc5
- v4.8-rc4
- v4.8-rc3
- v4.8-rc2
- v4.8-rc1
- v4.7
- v4.7-rc7
- v4.7-rc6
- v4.7-rc5
- v4.7-rc4
- v4.7-rc3
- v4.7-rc2
- v4.7-rc1
- v4.6
- v4.6-rc7
- v4.6-rc6
- v4.6-rc5
- v4.6-rc4
- v4.6-rc3
- v4.6-rc2
- v4.6-rc1
- v4.5
- v4.5-rc7
- v4.5-rc6
- v4.5-rc5
- v4.5-rc4
- v4.5-rc3
- v4.5-rc2
- v4.5-rc1
- v4.4
- v4.4-rc8
- v4.4-rc7
- v4.4-rc6
- v4.4-rc5
- v4.4-rc4
- v4.4-rc3
- v4.4-rc2
- v4.4-rc1
- v4.3
- v4.3-rc7
- v4.3-rc6
- v4.3-rc5
- v4.3-rc4
- v4.3-rc3
- v4.3-rc2
- v4.3-rc1
- v4.2
- v4.2-rc8
- v4.2-rc7
- v4.2-rc6
- v4.2-rc5
- v4.2-rc4
- v4.2-rc3
- v4.2-rc2
- v4.2-rc1
- v4.1
- v4.1-rc8
- v4.1-rc7
- v4.1-rc6
- v4.1-rc5
- v4.1-rc4
- v4.1-rc3
- v4.1-rc2
- v4.1-rc1
- v4.0
- v4.0-rc7
- v4.0-rc6
- v4.0-rc5
- v4.0-rc4
- v4.0-rc3
- v4.0-rc2
- v4.0-rc1
- v3.19
- v3.19-rc7
- v3.19-rc6
- v3.19-rc5
- v3.19-rc4
- v3.19-rc3
- v3.19-rc2
- v3.19-rc1
- v3.18
- v3.18-rc7
- v3.18-rc6
- v3.18-rc5
- v3.18-rc4
- v3.18-rc3
- v3.18-rc2
- v3.18-rc1
- v3.17
- v3.17-rc7
- v3.17-rc6
- v3.17-rc5
- v3.17-rc4
- v3.17-rc3
- v3.17-rc2
- v3.17-rc1
- v3.16
- v3.16-rc7
- v3.16-rc6
- v3.16-rc5
- v3.16-rc4
- v3.16-rc3
- v3.16-rc2
- v3.16-rc1
- v3.15
- v3.15-rc8
- v3.15-rc7
- v3.15-rc6
- v3.15-rc5
- v3.15-rc4
- v3.15-rc3
- v3.15-rc2
- v3.15-rc1
- v3.14
- v3.14-rc8
- v3.14-rc7
- v3.14-rc6
- v3.14-rc5
- v3.14-rc4
- v3.14-rc3
- v3.14-rc2
- v3.14-rc1
- v3.13
- v3.13-rc8
- v3.13-rc7
- v3.13-rc6
- v3.13-rc5
- v3.13-rc4
- v3.13-rc3
- v3.13-rc2
- v3.13-rc1
- v3.12
- v3.12-rc7
- v3.12-rc6
- v3.12-rc5
- v3.12-rc4
- v3.12-rc3
- v3.12-rc2
- v3.12-rc1
- v3.11
- v3.11-rc7
- v3.11-rc6
- v3.11-rc5
- v3.11-rc4
- v3.11-rc3
- v3.11-rc2
- v3.11-rc1
- v3.10
- v3.10-rc7
- v3.10-rc6
- v3.10-rc5
- v3.10-rc4
- v3.10-rc3
- v3.10-rc2
- v3.10-rc1
- v3.9
- v3.9-rc8
- v3.9-rc7
- v3.9-rc6
- v3.9-rc5
- v3.9-rc4
- v3.9-rc3
- v3.9-rc2
- v3.9-rc1
- v3.8
- v3.8-rc7
- v3.8-rc6
- v3.8-rc5
- v3.8-rc4
- v3.8-rc3
- v3.8-rc2
- v3.8-rc1
- v3.7
- v3.7-rc8
- v3.7-rc7
- v3.7-rc6
- v3.7-rc5
- v3.7-rc4
- v3.7-rc3
- v3.7-rc2
- v3.7-rc1
- v3.6
- v3.6-rc7
- v3.6-rc6
- v3.6-rc5
- v3.6-rc4
- v3.6-rc3
- v3.6-rc2
- v3.6-rc1
- v3.5
- v3.5-rc7
- v3.5-rc6
- v3.5-rc5
- v3.5-rc4
- v3.5-rc3
- v3.5-rc2
- v3.5-rc1
- v3.4
- v3.4-rc7
- v3.4-rc6
- v3.4-rc5
- v3.4-rc4
- v3.4-rc3
- v3.4-rc2
- v3.4-rc1
- v3.3
- v3.3-rc7
- v3.3-rc6
- v3.3-rc5
- v3.3-rc4
- v3.3-rc3
- v3.3-rc2
- v3.3-rc1
- v3.2
- v3.2-rc7
- v3.2-rc6
- v3.2-rc5
- v3.2-rc4
- v3.2-rc3
- v3.2-rc2
- v3.2-rc1
- v3.1
- v3.1-rc10
- v3.1-rc9
- v3.1-rc8
- v3.1-rc7
- v3.1-rc6
- v3.1-rc5
- v3.1-rc4
- v3.1-rc3
- v3.1-rc2
- v3.1-rc1
- v3.0
- v3.0-rc7
- v3.0-rc6
- v3.0-rc5
- v3.0-rc4
- v3.0-rc3
- v3.0-rc2
- v3.0-rc1
- v2.6.39
- v2.6.39-rc7
- v2.6.39-rc6
- v2.6.39-rc5
- v2.6.39-rc4
- v2.6.39-rc3
- v2.6.39-rc2
- v2.6.39-rc1
- v2.6.38
- v2.6.38-rc8
- v2.6.38-rc7
- v2.6.38-rc6
- v2.6.38-rc5
- v2.6.38-rc4
- v2.6.38-rc3
- v2.6.38-rc2
- v2.6.38-rc1
- v2.6.37
- v2.6.37-rc8
- v2.6.37-rc7
- v2.6.37-rc6
- v2.6.37-rc5
- v2.6.37-rc4
- v2.6.37-rc3
- v2.6.37-rc2
- v2.6.37-rc1
- v2.6.36
- v2.6.36-rc8
- v2.6.36-rc7
- v2.6.36-rc6
- v2.6.36-rc5
- v2.6.36-rc4
- v2.6.36-rc3
- v2.6.36-rc2
- v2.6.36-rc1
- v2.6.35
- v2.6.35-rc6
- v2.6.35-rc5
- v2.6.35-rc4
- v2.6.35-rc3
- v2.6.35-rc2
- v2.6.35-rc1
- v2.6.34
- v2.6.34-rc7
- v2.6.34-rc6
- v2.6.34-rc5
- v2.6.34-rc4
- v2.6.34-rc3
- v2.6.34-rc2
- v2.6.34-rc1
- v2.6.33
- v2.6.33-rc8
- v2.6.33-rc7
- v2.6.33-rc6
- v2.6.33-rc5
- v2.6.33-rc4
- v2.6.33-rc3
- v2.6.33-rc2
- v2.6.33-rc1
- v2.6.32
- v2.6.32-rc8
- v2.6.32-rc7
- v2.6.32-rc6
- v2.6.32-rc5
- v2.6.32-rc4
- v2.6.32-rc3
- v2.6.32-rc2
- v2.6.32-rc1
- v2.6.31
- v2.6.31-rc9
- v2.6.31-rc8
- v2.6.31-rc7
- v2.6.31-rc6
- v2.6.31-rc5
- v2.6.31-rc4
- v2.6.31-rc3
- v2.6.31-rc2
- v2.6.31-rc1
- v2.6.30
- v2.6.30-rc8
- v2.6.30-rc7
- v2.6.30-rc6
- v2.6.30-rc5
- v2.6.30-rc4
- v2.6.30-rc3
- v2.6.30-rc2
- v2.6.30-rc1
- v2.6.29
- v2.6.29-rc8
- v2.6.29-rc7
- v2.6.29-rc6
- v2.6.29-rc5
- v2.6.29-rc4
- v2.6.29-rc3
- v2.6.29-rc2
- v2.6.29-rc1
- v2.6.28
- v2.6.28-rc9
- v2.6.28-rc8
- v2.6.28-rc7
- v2.6.28-rc6
- v2.6.28-rc5
- v2.6.28-rc4
- v2.6.28-rc3
- v2.6.28-rc2
- v2.6.28-rc1
- v2.6.27
- v2.6.27-rc9
- v2.6.27-rc8
- v2.6.27-rc7
- v2.6.27-rc6
- v2.6.27-rc5
- v2.6.27-rc4
- v2.6.27-rc3
- v2.6.27-rc2
- v2.6.27-rc1
- v2.6.26
- v2.6.26-rc9
- v2.6.26-rc8
- v2.6.26-rc7
- v2.6.26-rc6
- v2.6.26-rc5
- v2.6.26-rc4
- v2.6.26-rc3
- v2.6.26-rc2
- v2.6.26-rc1
- v2.6.25
- v2.6.25-rc9
- v2.6.25-rc8
- v2.6.25-rc7
- v2.6.25-rc6
- v2.6.25-rc5
- v2.6.25-rc4
- v2.6.25-rc3
- v2.6.25-rc2
- v2.6.25-rc1
- v2.6.24
- v2.6.24-rc8
- v2.6.24-rc7
- v2.6.24-rc6
- v2.6.24-rc5
- v2.6.24-rc4
- v2.6.24-rc3
- v2.6.24-rc2
- v2.6.24-rc1
- v2.6.23
- v2.6.23-rc9
- v2.6.23-rc8
- v2.6.23-rc7
- v2.6.23-rc6
- v2.6.23-rc5
- v2.6.23-rc4
- v2.6.23-rc3
- v2.6.23-rc2
- v2.6.23-rc1
- v2.6.22
- v2.6.22-rc7
- v2.6.22-rc6
- v2.6.22-rc5
- v2.6.22-rc4
- v2.6.22-rc3
- v2.6.22-rc2
- v2.6.22-rc1
- v2.6.21
- v2.6.21-rc7
- v2.6.21-rc6
- v2.6.21-rc5
- v2.6.21-rc4
- v2.6.21-rc3
- v2.6.21-rc2
- v2.6.21-rc1
- v2.6.20
- v2.6.20-rc7
- v2.6.20-rc6
- v2.6.20-rc5
- v2.6.20-rc4
- v2.6.20-rc3
- v2.6.20-rc2
- v2.6.20-rc1
Showing
4 changed files
with
261 additions
and
357 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters