Received-SPF: pass (google.com: domain of linux-kernel+bounces-57553-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Ankit Agrawal <ankita@nvidia.com>, Alex Williamson
	<alex.williamson@redhat.com>, Zhi Wang <zhi.wang.linux@gmail.com>
CC: Jason Gunthorpe <jgg@nvidia.com>, Yishai Hadas <yishaih@nvidia.com>,
	"mst@redhat.com" <mst@redhat.com>, "shameerali.kolothum.thodi@huawei.com"
	<shameerali.kolothum.thodi@huawei.com>, "clg@redhat.com" <clg@redhat.com>,
	"oleksandr@natalenko.name" <oleksandr@natalenko.name>, "K V P, Satyanarayana"
	<satyanarayana.k.v.p@intel.com>, "eric.auger@redhat.com"
	<eric.auger@redhat.com>, "brett.creeley@amd.com" <brett.creeley@amd.com>,
	"horms@kernel.org" <horms@kernel.org>, Rahul Rameshbabu
	<rrameshbabu@nvidia.com>, Aniket Agashe <aniketa@nvidia.com>, Neo Jia
	<cjia@nvidia.com>, Kirti Wankhede <kwankhede@nvidia.com>, "Tarun Gupta
 (SW-GPU)" <targupta@nvidia.com>, Vikram Sethi <vsethi@nvidia.com>, "Currid,
 Andy" <acurrid@nvidia.com>, Alistair Popple <apopple@nvidia.com>, "John
 Hubbard" <jhubbard@nvidia.com>, Dan Williams <danw@nvidia.com>, "Anuj
 Aggarwal (SW-GPU)" <anuaggarwal@nvidia.com>, Matt Ochs <mochs@nvidia.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>
Subject: RE: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module for
 grace hopper
Thread-Topic: [PATCH v17 3/3] vfio/nvgrace-gpu: Add vfio pci variant module
 for grace hopper
Thread-Index: AQHaWIdk/6Inz9VYRUqD8voDMXYJJ7D/eesAgAAPggCAAIH/gIAAATfA
Date: Thu, 8 Feb 2024 07:21:40 +0000
Message-ID: <BN9PR11MB527640ADE99D92210977E7CA8C442@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20240205230123.18981-1-ankita@nvidia.com>
	<20240205230123.18981-4-ankita@nvidia.com>
	<20240208003210.000078ba.zhi.wang.linux@gmail.com>
 <20240207162740.1d713cf0.alex.williamson@redhat.com>
 <SA1PR12MB7199A9470EC2562C5BCC2FAFB0442@SA1PR12MB7199.namprd12.prod.outlook.com>
In-Reply-To: <SA1PR12MB7199A9470EC2562C5BCC2FAFB0442@SA1PR12MB7199.namprd12.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
msip_labels:
authentication-results: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=intel.com;
x-ms-publictraffictype: Email
x-ms-traffictypediagnostic: BN9PR11MB5276:EE_|CO1PR11MB4819:EE_
x-ms-office365-filtering-correlation-id: d298099b-987e-4388-6f79-08dc28769854
x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr
x-ms-exchange-senderadcheck: 1
x-ms-exchange-antispam-relay: 0
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: PLQunGWM6nS8DdWsui/GKWU//UaNuqZnaicCRUwqjAOD1tNbR4G8OcIGfp1QPecKzdlAHW8Ea9cWxTuqMTFA9ftzjEkPC3YLd52A+2BwrlDdZ9PLzJEqzIDb+n4Q6daRTVQDZKzfna+cZzqX02DMeKbif98ObPw3eHEIthzIiP4yN9YFXNtOWzHXpr6koDqScfBG/gK9TFoHVQRnmvdY0WESBItLb6kqxRx9Wqj8fZpteZNumfwLQPOjjKmbY+RAvjedHoS9H8tRSWAuMd00GZrruY6U2LU4ez2ek3r3StKiWDdGlDXyyS1MyF6gYtCDLhb0YKhepOSTWR3RPwJZDo6oPkMa+lq9xDiOXD4ZlHsM8w6seZXtIFEUqRthAmWzL5T15/YztfllPhE9edhxJcx8GTFVHbbf9TaOdW7XypICSb7XvzOkmxdUsewoXFK3wl6FHlCFjiSOI6dMtAAIS1dKFAh4d26OAZ7qGvhiwT8q5jrYuCcho6tqCY6vKvaDjqSv0SmGgwFqREKqVecSJlc4vxS0KsY5ohpqIQeW0LD70E4X9ISkDQtmmrUI2M7l6zkqsY2EU/dO+8pUI9tGZ+I1LrJslwxvw1lW6obiqkQ=
x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BN9PR11MB5276.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(366004)(39860400002)(396003)(376002)(136003)(346002)(230922051799003)(64100799003)(451199024)(186009)(1800799012)(7416002)(5660300002)(52536014)(2906002)(55016003)(41300700001)(83380400001)(26005)(122000001)(86362001)(38100700002)(6506007)(7696005)(110136005)(316002)(9686003)(8936002)(71200400001)(66446008)(478600001)(64756008)(76116006)(33656002)(66476007)(66556008)(66946007)(54906003)(82960400001)(4326008)(38070700009)(8676002);DIR:OUT;SFP:1101;
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?gW+YGVqck3o5RT4xJ379cEIKtG75QOjx76gwBHRXZxO/bCQTszLwRB0CNr?=
 =?iso-8859-1?Q?4GA5NoD/SAVin+XMkY9KI6nbQRKwM19xu6zoN4csW7PZG7s5KlkEMcFRw+?=
 =?iso-8859-1?Q?gLiDCXCHXK3dpZiGXRJXlYclenk63+OGk+wQiC4plPO9WAHTB5bgw/RTwO?=
 =?iso-8859-1?Q?6qF3l722QcOVd7KlSPUbQlc7ZF/LHt05uM24EMx9wbRukqV80vXKQJ/b/R?=
 =?iso-8859-1?Q?HdBg6n0LAymIAtBHVoPpUWLLXJvr16oJYrCp9AutFt5lW6qZZUYxXtNkiF?=
 =?iso-8859-1?Q?cL8x4nrUf2EOKvIxlUZpoP4qnKAe4R+ZIPh4BFtpFuegpl05AUrmCc0Sxo?=
 =?iso-8859-1?Q?nlS7+7bTm+OzsQv2zaUqzZMe9s1Lgu497ydsgg6ae7lyeSil0qKzyzMRSt?=
 =?iso-8859-1?Q?iyxyhaZVXDzJBU6cSMX6pmg999N349Yc+C9eK/YidkfyJUb4XCZSdGSK2A?=
 =?iso-8859-1?Q?ojDPK9rv08UX6C1vkFHbNZc+oZsb+OWO7WhhBbUOcht/h1aeLsgf6zpPQg?=
 =?iso-8859-1?Q?tqmmEPogqPCHrNCo3z4HaDombSkRmiajwXbphi3Q3zf6pX9PEEnHYlSi14?=
 =?iso-8859-1?Q?i0oVboRDuJFtO0pIWyq4EG4ODMDNcMw5JisvUw4QJeqOPzgNTORfw3f76Q?=
 =?iso-8859-1?Q?7WAKNMmU3D8OJ6JjD39pmwvtQFrweHYcyNlQbCSfy63Ri3u0hdnzkCgo77?=
 =?iso-8859-1?Q?wzpYLg6CyLN+b2dsZ//IJsYEsVJktCl1nEgg93TsHixQOlxYJQGVzTvc5W?=
 =?iso-8859-1?Q?2x6tj281i5N5J+XCu2tsmeNitzguZLWKv+FD2SBvIfvoa+EZoVjwP9eh6X?=
 =?iso-8859-1?Q?gHhMcHwRl9aW8C0qm9U1CFvjFz13O3CVImNz/ec9DcZhzjDoGW1tyGl6gL?=
 =?iso-8859-1?Q?iApSHlTLWCogjZOfN2jCHitKyqX5MtxtzZUjIr3dPP+EdJzUDlO7duqTJ8?=
 =?iso-8859-1?Q?/+rlFBrKwX2W8+qkgcP/HRFZab7KIqtHwYbgznshPGKEKOvfCwoPRmi/Et?=
 =?iso-8859-1?Q?EmVTL5Xg5I//mLJR8e5+327WWbXTr6WoWzve9pniiDjqsLQnt5Kn0/AXQ4?=
 =?iso-8859-1?Q?DDjPUKFUGmPYQB/3DyLkitmEecIDgFo5o4BOnOfxZER7twyrBmWkJgrgEy?=
 =?iso-8859-1?Q?A2hM96Q6Wt6WrgAGWs8OF/pEETUaj1rdMZnAyhFrCZW/sQrcPevQ0lAQ2h?=
 =?iso-8859-1?Q?QCicudjOcEeyCiyDpsT4qvI+I7eusMwNwBR8201voxtOReNSz8uy9NDIRV?=
 =?iso-8859-1?Q?GGjW6TTRtSEM8Y3QJb1ui9B+RECuL/CyUXR38lr/oLUP/CoZgeuCqN2uiJ?=
 =?iso-8859-1?Q?zaXBr4uFMUsa35WpCibKqp+nImwwFcUmk7Sj6Og+IVXwKmfI7+suYjU2iI?=
 =?iso-8859-1?Q?/TjMZy9vVLzyFUNY4CQ2/a7I5Fig2+ZrO4x7alMBUmxNE5jE4j0YKhTZph?=
 =?iso-8859-1?Q?gGqsMGNUyDydPHknvxkKKyRiMTof/rcy0f/nS5MRczgxn1jnf77qDp3YVp?=
 =?iso-8859-1?Q?gTieMY7C43bKuwSg7aW6FT2X2l7v/xUyLS27i0So0bQ5ZeB25EKVEAc6LJ?=
 =?iso-8859-1?Q?UtURksT9EJ1mGokAwrNGmYLKVAqpUDloVZ85VYMwCr6HogO5e3fYrPI0PB?=
 =?iso-8859-1?Q?iEkr0wBUuoZJ1KV/Ps9yadvOfamjDlZSx9?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5276.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d298099b-987e-4388-6f79-08dc28769854
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Feb 2024 07:21:40.6066
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: OL+zKlqhZ/u81aNFkNWILX70t0lVvoCJE1eNHwl9B55siZCifaCpJJk+EBlwdfjWGKMdJrll4NpK8kudq+D1mg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB4819
X-OriginatorOrg: intel.com

> From: Ankit Agrawal <ankita@nvidia.com>
> Sent: Thursday, February 8, 2024 3:13 PM
> >> > +=A0=A0=A0 * Determine how many bytes to be actually read from the
> >> > device memory.
> >> > +=A0=A0=A0 * Read request beyond the actual device memory size is
> >> > filled with ~0,
> >> > +=A0=A0=A0 * while those beyond the actual reported size is skipped.
> >> > +=A0=A0=A0 */
> >> > +=A0=A0 if (offset >=3D memregion->memlength)
> >> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 mem_count =3D 0;
> >>
> >> If mem_count =3D=3D 0, going through nvgrace_gpu_map_and_read() is not
> >> necessary.
> >
> > Harmless, other than the possibly unnecessary call through to
> > nvgrace_gpu_map_device_mem().=A0 Maybe both
> nvgrace_gpu_map_and_read()
> > and nvgrace_gpu_map_and_write() could conditionally return 0 as their
> > first operation when !mem_count.=A0 Thanks,
> >
> >Alex
>=20
> IMO, this seems like adding too much code to reduce the call length for a
> very specific case. If there aren't any strong opinion on this, I'm plann=
ing to
> leave this code as it is.

a slight difference. if mem_count=3D=3D0 the result should always succeed
no matter nvgrace_gpu_map_device_mem() succeeds or not. Of course
if it fails it's already a big problem probably nobody cares about the subt=
le
difference when reading non-exist range.

but regarding to readability it's still clearer:

if (mem_count)
	nvgrace_gpu_map_and_read();