On Wed, Jul 23, 2014 at 01:04:35PM +0800, Kinglong Mee wrote:
> On 7/21/2014 23:55, J. Bruce Fields wrote:
> > On Sat, Jul 19, 2014 at 11:23:59AM +0800, Kinglong Mee wrote:
> >> On Sat, Jul 19, 2014 at 12:50 AM, Toralf Förster <toralf.foerster@xxxxxx> wrote:
> >>> On 07/18/2014 06:22 PM, Toralf Förster wrote:
> >>>> I can now try with kzalloc, but due to the nature of this issue I think, that the absence of this crash - even after 2-3 hours - doesn't mean by 100%, that kzalloc fixed it, or ?
> >>>
> >>> Well, next crash (with kzalloc patch) happened after 20 minutes ...
> >>
> >> Maybe I have found the problem.
> >> The stateid and denied are defined as an union as,
> >> fs/nfsd/xdr4.h
> >> 145 struct nfsd4_lock_denied {
> >> 146 clientid_t ld_clientid;
> >> 147 struct xdr_netobj ld_owner;
> >> 148 u64 ld_start;
> >> 149 u64 ld_length;
> >> 150 u32 ld_type;
> >> 151 };
> >> 152
> >> 153 struct nfsd4_lock {
> >> ... ...
> >> 174 /* response */
> >> 175 union {
> >> 176 struct {
> >> 177 stateid_t stateid;
> >> 178 } ok;
> >> 179 struct nfsd4_lock_denied denied;
> >> 180 } u;
> >>
> >> 30 struct xdr_netobj {
> >> 31 unsigned int len;
> >> 32 u8 * data;
> >> 33 };
> >>
> >> sizeof(stateid_t) = 16, sizeof(clientid_t) = 8,
> >> sizeof(struct xdr_netobj) = 16, (on x86_x64 platform),
> >> sizeof(struct xdr_netobj) = 8, (on i686 platform)
> >>
> >> Lock file success, nfsd will copy stateid to the union, but the value
> >> also influence denied.
> >> If on x86_64 platform, only influence the len in xdr_netobj,
> >> but on i686 platform, will influence the len and the data in xdr_netobj.
> >> So, the problem only appears on i686 platform.
> >
> > Oh, great catch, thanks. Sounds like that would explain all of Toralf's
> > results.
> >
> > I'll include this explanation with your original patch and submit it for
> > 3.16.
>
> I saw the patch in your git-tree, there is one fault about the comments.
>
> > (Note that lock->lk_denied.ld_owner.data appears it should be zero here,
> > until you notice that it's one arm of a union the other arm of which is
> > written to in the succesful case by the
> >
> > memcpy(&oc->oc_resp_stateid, &stp->st_stid.sc_stateid, sizeof(stateid_t));
> >
> > in nfsd4_open_confirm().
>
> It's,
> memcpy(&lock->lk_resp_stateid, &lock_stp->st_stid.sc_stateid,
> sizeof(stateid_t));
> in nfsd4_lock().
Thanks, fixed!
--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
