From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f179.google.com (mail-pl1-f179.google.com [209.85.214.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 92703201264 for ; Wed, 8 Jan 2025 18:40:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736361630; cv=none; b=jf1AJDsXcmkQMAK7z4Z7Iahbq1/f+E3u74E53rjhbbYW83iEvEYCYGbfjcyoOaefCnEofg2RLW9KUBmCuN1c/B+yVFMPmFJ5Hd33+yAYnwJ2qB8Eiyj/NfhaudAwiBeOtTC+ih5icJJRCL5csC6+M0AVLwzWZHpOvFxuNOdf4j4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736361630; c=relaxed/simple; bh=VqlHoOop0NCh6IK9DagrNkCPKNyMG6x3Ydtmn6STuAk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=F6FnxVYwZlhOTsaCoilHjTudiPqS1V4P/SAGEQG+Ih3QIp0SKhjZdUa/nFlA2orW0l3d3C9zrT97LhKbZN2HyHnrh7/wl2LcIIH/OQSD+iqIHMMPDoSVZeWQDzdtrvrQsIAjMPi5lHJUGcVUDKtO/IBj4psIpaS4v5LZcqDb0cI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=hWez+H6e; arc=none smtp.client-ip=209.85.214.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="hWez+H6e" Received: by mail-pl1-f179.google.com with SMTP id d9443c01a7336-215740b7fb8so11965ad.0 for ; Wed, 08 Jan 2025 10:40:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1736361628; x=1736966428; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=MtXSgCoPknXUV18KTNBhK6DwFGfPYSbawI86wtxvN+U=; b=hWez+H6eFj+vHTls1hkjoq3IIPFF/+UAsSfQSdQ/9qLFDPkrHvnTqgeKR6/YtIpX+F iaPo6d6a8c2rOirjmtbqrrvKERlpxP5akqu9cLS2PhW4aJ+ow4YbUSABPKquIMyvNFuc v2Zx/VwEx2Bx6UUfoCnKFGCOeuZvwZA8RzyeUAAM+nkqWrv2vo3jjSileyvM67qfIh+O tBTfXx51HFWOO8ybB0APxOX1Xct6K5TfNHVMs4FSPT17nKq4iCNHvzWTtCLdQ9MnYwbR aMv5NnXWVqdCpH7fgrQqHuCns2CVOfnwtcNyLRR282E/xUGWm3bURyvccQxj1pGH80kG mPJA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736361628; x=1736966428; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=MtXSgCoPknXUV18KTNBhK6DwFGfPYSbawI86wtxvN+U=; b=MGPMNl4VFje7qN1hqc14424Ak5bS/w77uNZXIx4k29LBMgPg9z5/XzrNkU8TIrh39p I8JzuZUmCH0+QgtBdDffMhbH0IkanjfNZPg5qdWssG+K3p6Mv76Gxswa9onYb8jUxCW6 XzC4ppO+DH2K167i8y9cTBwtI+p6J1d+2ir2Yhvp3IzlwesRQHQfijnmksLw5neLoF/a aEaGCkuXCJUV40ypRPKfoygskP4KwFlSqtx7smMEKVA9g5HJ2SMXPKvWg0Xy50LyvjnV PM4S+FbDLiiNsP3HGbt+jagi1RJSYyXlSt0GEwOdAZd/JH6XvW1gpPlylCpe42uvPoCN 5iOw== X-Forwarded-Encrypted: i=1; AJvYcCVAj9iUiVRioaLF2kMj2ZPWy8fEHKfkXSOKDPPvYVSQq9j5REBEo9EGa6i9qY3omXDw4nthm8cMQIwrje0=@vger.kernel.org X-Gm-Message-State: AOJu0YzxD4ZIpUAvCSksPuNr28LKAO5VfaCq3G06TpN+U1ngfoVBT9hP F1+IRdlTI1i/IobkICfaqzFkt9DS7dNR6ezlp91uFBOD2eLHia6YtRfTH4uiew== X-Gm-Gg: ASbGncvpQ0mNZtw49IgCKrXzlgu/MOaGKHj1rZ4UXmuDKYlgMQ0FLpOaKSplQ12ZgIf QY4drc/Vm814u4F9mNCMrBkpGCPOyt2jAFK3VHvhycMfrjtahgYgdHbThhft/XyZHif3HGU+WpO kbZJxAnDz6RUmB79dAhifFx3Z3J++buqpOq6n09RC9IzB2zIAmkXthf2UKh4dVs28gvh/ndHaXY 9HURjmqrsxXSi8jFGGCmVMNtRsNsEJic5CtKy7QKr/6cYN32lXsWDWG X-Google-Smtp-Source: AGHT+IGyT4yIZnpuUdJ+unZ/TVEwIeNLMGKjSePXMeLbBRfO0FLc2xjsqLeQ+b2RLyKS/+8GNGEOXQ== X-Received: by 2002:a17:902:d484:b0:215:772c:fa82 with SMTP id d9443c01a7336-21a853bb5f6mr2546675ad.3.1736361627492; Wed, 08 Jan 2025 10:40:27 -0800 (PST) Received: from google.com ([2620:15c:2d:3:e514:4b22:2740:5ec1]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-219dc9d9481sm329138945ad.135.2025.01.08.10.40.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Jan 2025 10:40:26 -0800 (PST) Date: Wed, 8 Jan 2025 10:40:22 -0800 From: Isaac Manjarres To: Alice Ryhl Cc: lorenzo.stoakes@oracle.com, Andrew Morton , kaleshsingh@google.com, jstultz@google.com, surenb@google.com, kernel-team@android.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/2] mm/memfd: Refactor and cleanup the logic in memfd_create() Message-ID: References: <20250107184804.4074147-1-isaacmanjarres@google.com> <20250107184804.4074147-2-isaacmanjarres@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Wed, Jan 08, 2025 at 02:31:58PM +0100, Alice Ryhl wrote: > On Tue, Jan 7, 2025 at 7:48 PM Isaac J. Manjarres > wrote: > > +SYSCALL_DEFINE2(memfd_create, > > + const char __user *, uname, > > + unsigned int, flags) > > +{ > > + struct file *file; > > + int fd; > > + char *name; > > + > > + name = memfd_create_name(uname); > > + if (IS_ERR(name)) > > + return PTR_ERR(name); > > + > > + file = memfd_file_create(name, flags); > > + /* name is not needed beyond this point. */ > > kfree(name); > > - return error; > > + if (IS_ERR(file)) > > + return PTR_ERR(file); > > + > > + fd = get_unused_fd_flags((flags & MFD_CLOEXEC) ? O_CLOEXEC : 0); > > + if (fd >= 0) > > + fd_install(fd, file); > > + else > > + fput(file); > > You changed the order so that get_unused_fd_flags() happens after > creating the file, so the error path now does fput(file) instead of > put_unused_fd(fd). Is there a reason for this? I would generally > assume that calling get_unused_fd_flags() first is better. Thanks for taking a look at this, Alice! I changed the order so that the code had a more logical structure where we create objects and use them right away, as opposed to getting an fd, then creating the file to associate with that file descriptor, and then actually associating it. I also structured the code to get rid of the gotos in this function to make it easier to follow. It also made sense to me to fold the flags validation into memfd_file_create() since that's where the flags are used the most anyway, and it also makes sense to validate the flags first, so reordering the file creation and fd creation allowed me to do that. I'm open to restoring the order back to how it was though. Is there a reason for why get_unused_fd_flags() first is better? Thanks, Isaac