Re: [PATCH 2/3] xfs: refactor unmount record write

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Jul 19, 2018 at 09:39:47AM -0700, Christoph Hellwig wrote:
> Looks generally fine, but some tiny nitpicks:
> 
> > +	/* the data section must be 32 bit size aligned */
> > +	struct {
> > +		uint16_t magic;
> > +		uint16_t pad1;
> > +		uint32_t pad2; /* may as well make it 64 bits */
> > +	} magic = {
> > +		.magic = XLOG_UNMOUNT_TYPE,
> > +	};
> 
> Can we please give this structure type a name and move it to
> xfs_log_format.h?  They way this had been done always irked me.

Ok.  It gets written to disk, so that makes sense.

> > +	/*
> > +	 * At this point, we're umounting anyway,
> > +	 * so there's no point in transitioning log state
> > +	 * to IOERROR. Just continue...
> > +	 */
> 
> Use up all 80 lines, please.
> 
> > +	if (!(iclog->ic_state == XLOG_STATE_ACTIVE ||
> > +	      iclog->ic_state == XLOG_STATE_DIRTY) &&
> > +	    !XLOG_FORCED_SHUTDOWN(log))
> > +		xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
> > +	else
> > +		spin_unlock(&log->l_icloglock);
> 
> 	switch (iclog->ic_state) {
> 	default:
> 		if (!XLOG_FORCED_SHUTDOWN(log)) {
> 			xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
> 			break;
> 		}
> 		/*FALLHRU*/
> 	case XLOG_STATE_ACTIVE:
> 	case XLOG_STATE_DIRTY:
> 		spin_unlock(&log->l_icloglock);
> 		break;

<nod>

--D

> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html