From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bob Peterson Date: Fri, 25 Aug 2017 12:06:49 -0400 (EDT) Subject: [Cluster-devel] [bug report] GFS2: Withdraw for IO errors writing to the journal or statfs In-Reply-To: <20170824122330.GA16546@cicero> References: <20170824110357.b3t7z2wcnl7doeyt@mwanda> <20170824122330.GA16546@cicero> Message-ID: <742248365.1949596.1503677209523.JavaMail.zimbra@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit ----- Original Message ----- | On Thu, Aug 24, 2017 at 02:03:57PM +0300, Dan Carpenter wrote: | > Hello Bob Peterson, | > | > The patch 9bfef7554e20: "GFS2: Withdraw for IO errors writing to the | > journal or statfs" from Aug 16, 2017, leads to the following static | > checker warning: | > | > fs/gfs2/super.c:949 gfs2_sync_fs() | > error: we previously assumed 'sdp' could be null (see line 947) | > | > fs/gfs2/super.c | > 942 static int gfs2_sync_fs(struct super_block *sb, int wait) | > 943 { | > 944 struct gfs2_sbd *sdp = sb->s_fs_info; | > 945 | > 946 gfs2_quota_sync(sb, -1); | > 947 if (wait && sdp) | > ^^^ | > Existing code checks for NULL. | | The NULL check seemed odd to me, and other ->sync_fs implementations don't | check it, but digging through the history there's a reason it was added (see | 9171f5a ). That said, I can't see the quota_off code path that it was | guarding against any more, so perhaps it is no longer required. | | Andy | | > | > 948 gfs2_log_flush(sdp, NULL, NORMAL_FLUSH); | > 949 return sdp->sd_log_error; | > ^^^^^^^^^^^^^^^^^ | > Patch adds unchecked dereference. | Hi Andy (and Dan), You're right. I did some research and this check for "&& sdp" seems completely unnecessary now. In fact, I took it out and tried to recreate the problem for which it was added (mounting with an invalid lock protocol) and it did not recreate. Therefore, I removed the offending check and did a force-push back to for-next. Thanks; well spotted. Regards, Bob Peterson Red Hat File Systems