tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: vn_open, EDUPFD, EMOVEFD



On Tue, Jun 29, 2021 at 05:07:10AM -0000, Christos Zoulas wrote:
 > >* Does anyone know why dev/kloader.c calls namei and then vn_open on
 > >the same path? I remember seeing this before and leaving it alone
 > >because nobody I could find was sure what the deal was, but it's
 > >unlikely to work as-is and increasingly likely to break over time...
 > 
 > Just fix it, and if something breaks we'll put it back.

Ok, I will do exactly that :-) but separately.

 > >-	NDINIT(&nd, LOOKUP, NOFOLLOW, pb);
 > >-	error = vn_open(&nd, filemode, createmode);
 > >+	error = vn_open(NULL, pb, 0, filemode, createmode, vpp, NULL, NULL);
 > 
 > This is the only NOFOLLOW NDINIT case, should that be 'createmode |
 > O_NOFOLLOW'?

Yes it should, thanks. (Except filemode, not createmode.) Too many
mostly-the-same changes...

 > >-	NDINIT(&nd, CREATE, LOCKPARENT, pb);
 > > 	
 > > 	/*
 > > 	 * Since we do not hold ulfs_extattr_uepm_lock anymore,
 > > 	 * another thread may race with us for backend creation,
 > >-	 * but only one can succeed here thanks to O_EXCL
 > >+	 * but only one can succeed here thanks to O_EXCL.
 > >+	 *
 > >+ 	 * backing_vp is the backing store. 
 > > 	 */
 > >-	error = vn_open(&nd, O_CREAT|O_EXCL|O_RDWR, 0600);
 > >+	error = vn_open(NULL, pb, 0, O_CREAT|O_EXCL|O_RDWR, 0600,
 > >+			&backing_vp, NULL, NULL);
 > 
 > I guess O_CREAT will do the LOCKPARENT later...

Indeed.

 > I would have preferred if EMOVEFD/EDUPFD were gc'ed as part of the patch,
 > because there is a lot of ugliness left.

So would I, but it's a big deal (requires changing every [cb]devsw...)

-- 
David A. Holland
dholland%netbsd.org@localhost


Home | Main Index | Thread Index | Old Index