| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1885c10881b04c4083e8742082d68b36@huawei.com>
Date: Fri, 22 Nov 2024 10:31:38 +0000
From: Shiju Jose <shiju.jose@...wei.com>
To: Jonathan Cameron <jonathan.cameron@...wei.com>
CC: "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
"linux-cxl@...r.kernel.org" <linux-cxl@...r.kernel.org>, "mchehab@...nel.org"
<mchehab@...nel.org>, "dave.jiang@...el.com" <dave.jiang@...el.com>,
"dan.j.williams@...el.com" <dan.j.williams@...el.com>,
"alison.schofield@...el.com" <alison.schofield@...el.com>,
"nifan.cxl@...il.com" <nifan.cxl@...il.com>, "vishal.l.verma@...el.com"
<vishal.l.verma@...el.com>, "ira.weiny@...el.com" <ira.weiny@...el.com>,
"dave@...olabs.net" <dave@...olabs.net>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, Linuxarm <linuxarm@...wei.com>, tanxiaofei
<tanxiaofei@...wei.com>, "Zengtao (B)" <prime.zeng@...ilicon.com>
Subject: RE: [PATCH 03/13] rasdaemon: cxl: Add automatic indexing for storing
CXL fields in SQLite database
>-----Original Message-----
>From: Jonathan Cameron <jonathan.cameron@...wei.com>
>Sent: 21 November 2024 15:17
>To: Shiju Jose <shiju.jose@...wei.com>
>Cc: linux-edac@...r.kernel.org; linux-cxl@...r.kernel.org;
>mchehab@...nel.org; dave.jiang@...el.com; dan.j.williams@...el.com;
>alison.schofield@...el.com; nifan.cxl@...il.com; vishal.l.verma@...el.com;
>ira.weiny@...el.com; dave@...olabs.net; linux-kernel@...r.kernel.org;
>Linuxarm <linuxarm@...wei.com>; tanxiaofei <tanxiaofei@...wei.com>;
>Zengtao (B) <prime.zeng@...ilicon.com>
>Subject: Re: [PATCH 03/13] rasdaemon: cxl: Add automatic indexing for storing
>CXL fields in SQLite database
>
>On Wed, 20 Nov 2024 09:59:13 +0000
><shiju.jose@...wei.com> wrote:
>
>> From: Shiju Jose <shiju.jose@...wei.com>
>>
>> When the CXL specification adds new fields to the common header of CXL
>> event records, manual updates to the indexing are required to store
>> these CXL fields in the SQLite database. This update introduces
>> automatic indexing to facilitate the storage of CXL fields in the
>> SQLite database, eliminating the need for manual update to indexing.
>>
>> Signed-off-by: Shiju Jose <shiju.jose@...wei.com>
>
>Using 0 as an error code seems odd, maybe a negative instead?
>With that changed to say -1 then this looks good to me.
Will change error code to -1
Thanks,
Shiju
>
>Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
>
>
>> ---
>> ras-record.c | 134
>> ++++++++++++++++++++++++++++-----------------------
>> 1 file changed, 74 insertions(+), 60 deletions(-)
>>
>> diff --git a/ras-record.c b/ras-record.c index b4a1abd..9e68158 100644
>> --- a/ras-record.c
>> +++ b/ras-record.c
>> @@ -780,23 +780,25 @@ int ras_store_cxl_overflow_event(struct
>> ras_events *ras, struct ras_cxl_overflow
>>
>> static int ras_store_cxl_common_hdr(sqlite3_stmt *stmt, struct
>> ras_cxl_event_common_hdr *hdr) {
>> + int idx = 1;
>> +
>> if (!stmt || !hdr)
>> return 0;
>>
>> - sqlite3_bind_text(stmt, 1, hdr->timestamp, -1, NULL);
>> - sqlite3_bind_text(stmt, 2, hdr->memdev, -1, NULL);
>> - sqlite3_bind_text(stmt, 3, hdr->host, -1, NULL);
>> - sqlite3_bind_int64(stmt, 4, hdr->serial);
>> - sqlite3_bind_text(stmt, 5, hdr->log_type, -1, NULL);
>> - sqlite3_bind_text(stmt, 6, hdr->hdr_uuid, -1, NULL);
>> - sqlite3_bind_int(stmt, 7, hdr->hdr_flags);
>> - sqlite3_bind_int(stmt, 8, hdr->hdr_handle);
>> - sqlite3_bind_int(stmt, 9, hdr->hdr_related_handle);
>> - sqlite3_bind_text(stmt, 10, hdr->hdr_timestamp, -1, NULL);
>> - sqlite3_bind_int(stmt, 11, hdr->hdr_length);
>> - sqlite3_bind_int(stmt, 12, hdr->hdr_maint_op_class);
>> -
>> - return 0;
>> + sqlite3_bind_text(stmt, idx++, hdr->timestamp, -1, NULL);
>> + sqlite3_bind_text(stmt, idx++, hdr->memdev, -1, NULL);
>> + sqlite3_bind_text(stmt, idx++, hdr->host, -1, NULL);
>> + sqlite3_bind_int64(stmt, idx++, hdr->serial);
>> + sqlite3_bind_text(stmt, idx++, hdr->log_type, -1, NULL);
>> + sqlite3_bind_text(stmt, idx++, hdr->hdr_uuid, -1, NULL);
>> + sqlite3_bind_int(stmt, idx++, hdr->hdr_flags);
>> + sqlite3_bind_int(stmt, idx++, hdr->hdr_handle);
>> + sqlite3_bind_int(stmt, idx++, hdr->hdr_related_handle);
>> + sqlite3_bind_text(stmt, idx++, hdr->hdr_timestamp, -1, NULL);
>> + sqlite3_bind_int(stmt, idx++, hdr->hdr_length);
>> + sqlite3_bind_int(stmt, idx++, hdr->hdr_maint_op_class);
>> +
>> + return idx;
>> }
>>
>> /*
>> @@ -827,15 +829,18 @@ static const struct db_table_descriptor
>> cxl_generic_event_tab = {
>>
>> int ras_store_cxl_generic_event(struct ras_events *ras, struct
>> ras_cxl_generic_event *ev) {
>> - int rc;
>> + int rc, idx;
>> struct sqlite3_priv *priv = ras->db_priv;
>>
>> if (!priv || !priv->stmt_cxl_generic_event)
>> return 0;
>> log(TERM, LOG_INFO, "cxl_generic_event store: %p\n",
>> priv->stmt_cxl_generic_event);
>>
>> - ras_store_cxl_common_hdr(priv->stmt_cxl_generic_event, &ev->hdr);
>> - sqlite3_bind_blob(priv->stmt_cxl_generic_event, 13, ev->data,
>> + idx = ras_store_cxl_common_hdr(priv->stmt_cxl_generic_event, &ev-
>>hdr);
>> + if (!idx)
>> + return 0;
>Seems like an odd form of error code as without looking at implementation it
>might seem reasonable for that call to return 0 because it didn't add anythin to
>idx?
>
>> +
>> + sqlite3_bind_blob(priv->stmt_cxl_generic_event, idx++, ev->data,
>> CXL_EVENT_RECORD_DATA_LENGTH, NULL);
>>
>> rc = sqlite3_step(priv->stmt_cxl_generic_event);
>> @@ -891,7 +896,7 @@ static const struct db_table_descriptor
>> cxl_general_media_event_tab = { int
>ras_store_cxl_general_media_event(struct ras_events *ras,
>> struct ras_cxl_general_media_event *ev) {
>> - int rc;
>> + int rc, idx;
>> struct sqlite3_priv *priv = ras->db_priv;
>>
>> if (!priv || !priv->stmt_cxl_general_media_event)
>> @@ -899,20 +904,23 @@ int ras_store_cxl_general_media_event(struct
>ras_events *ras,
>> log(TERM, LOG_INFO, "cxl_general_media_event store: %p\n",
>> priv->stmt_cxl_general_media_event);
>>
>> - ras_store_cxl_common_hdr(priv->stmt_cxl_general_media_event, &ev-
>>hdr);
>> - sqlite3_bind_int64(priv->stmt_cxl_general_media_event, 13, ev->dpa);
>> - sqlite3_bind_int(priv->stmt_cxl_general_media_event, 14, ev-
>>dpa_flags);
>> - sqlite3_bind_int(priv->stmt_cxl_general_media_event, 15, ev-
>>descriptor);
>> - sqlite3_bind_int(priv->stmt_cxl_general_media_event, 16, ev->type);
>> - sqlite3_bind_int(priv->stmt_cxl_general_media_event, 17, ev-
>>transaction_type);
>> - sqlite3_bind_int(priv->stmt_cxl_general_media_event, 18, ev->channel);
>> - sqlite3_bind_int(priv->stmt_cxl_general_media_event, 19, ev->rank);
>> - sqlite3_bind_int(priv->stmt_cxl_general_media_event, 20, ev->device);
>> - sqlite3_bind_blob(priv->stmt_cxl_general_media_event, 21, ev-
>>comp_id,
>> + idx = ras_store_cxl_common_hdr(priv->stmt_cxl_general_media_event,
>&ev->hdr);
>> + if (!idx)
>As above,
>> + return 0;
>> +
>> + sqlite3_bind_int64(priv->stmt_cxl_general_media_event, idx++, ev-
>>dpa);
>> + sqlite3_bind_int(priv->stmt_cxl_general_media_event, idx++, ev-
>>dpa_flags);
>> + sqlite3_bind_int(priv->stmt_cxl_general_media_event, idx++, ev-
>>descriptor);
>> + sqlite3_bind_int(priv->stmt_cxl_general_media_event, idx++, ev->type);
>> + sqlite3_bind_int(priv->stmt_cxl_general_media_event, idx++, ev-
>>transaction_type);
>> + sqlite3_bind_int(priv->stmt_cxl_general_media_event, idx++, ev-
>>channel);
>> + sqlite3_bind_int(priv->stmt_cxl_general_media_event, idx++, ev->rank);
>> + sqlite3_bind_int(priv->stmt_cxl_general_media_event, idx++, ev-
>>device);
>> + sqlite3_bind_blob(priv->stmt_cxl_general_media_event, idx++,
>> +ev->comp_id,
>> CXL_EVENT_GEN_MED_COMP_ID_SIZE, NULL);
>> - sqlite3_bind_int64(priv->stmt_cxl_general_media_event, 22, ev->hpa);
>> - sqlite3_bind_text(priv->stmt_cxl_general_media_event, 23, ev->region,
>-1, NULL);
>> - sqlite3_bind_text(priv->stmt_cxl_general_media_event, 24, ev-
>>region_uuid, -1, NULL);
>> + sqlite3_bind_int64(priv->stmt_cxl_general_media_event, idx++, ev-
>>hpa);
>> + sqlite3_bind_text(priv->stmt_cxl_general_media_event, idx++, ev-
>>region, -1, NULL);
>> + sqlite3_bind_text(priv->stmt_cxl_general_media_event, idx++,
>> +ev->region_uuid, -1, NULL);
>>
>> rc = sqlite3_step(priv->stmt_cxl_general_media_event);
>> if (rc != SQLITE_OK && rc != SQLITE_DONE) @@ -970,7 +978,7 @@
>static
>> const struct db_table_descriptor cxl_dram_event_tab = {
>>
>> int ras_store_cxl_dram_event(struct ras_events *ras, struct
>> ras_cxl_dram_event *ev) {
>> - int rc;
>> + int rc, idx;
>> struct sqlite3_priv *priv = ras->db_priv;
>>
>> if (!priv || !priv->stmt_cxl_dram_event) @@ -978,24 +986,27 @@ int
>> ras_store_cxl_dram_event(struct ras_events *ras, struct ras_cxl_dram_event
>*
>> log(TERM, LOG_INFO, "cxl_dram_event store: %p\n",
>> priv->stmt_cxl_dram_event);
>>
>> - ras_store_cxl_common_hdr(priv->stmt_cxl_dram_event, &ev->hdr);
>> - sqlite3_bind_int64(priv->stmt_cxl_dram_event, 13, ev->dpa);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 14, ev->dpa_flags);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 15, ev->descriptor);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 16, ev->type);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 17, ev->transaction_type);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 18, ev->channel);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 19, ev->rank);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 20, ev->nibble_mask);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 21, ev->bank_group);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 22, ev->bank);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 23, ev->row);
>> - sqlite3_bind_int(priv->stmt_cxl_dram_event, 24, ev->column);
>> - sqlite3_bind_blob(priv->stmt_cxl_dram_event, 25, ev->cor_mask,
>> + idx = ras_store_cxl_common_hdr(priv->stmt_cxl_dram_event, &ev-
>>hdr);
>> + if (!idx)
>As above.
>
>> + return 0;
>> +
>> + sqlite3_bind_int64(priv->stmt_cxl_dram_event, idx++, ev->dpa);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev->dpa_flags);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev->descriptor);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev->type);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev-
>>transaction_type);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev->channel);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev->rank);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev->nibble_mask);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev->bank_group);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev->bank);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev->row);
>> + sqlite3_bind_int(priv->stmt_cxl_dram_event, idx++, ev->column);
>> + sqlite3_bind_blob(priv->stmt_cxl_dram_event, idx++, ev->cor_mask,
>> CXL_EVENT_DER_CORRECTION_MASK_SIZE, NULL);
>> - sqlite3_bind_int64(priv->stmt_cxl_dram_event, 26, ev->hpa);
>> - sqlite3_bind_text(priv->stmt_cxl_dram_event, 27, ev->region, -1, NULL);
>> - sqlite3_bind_text(priv->stmt_cxl_dram_event, 28, ev->region_uuid, -1,
>NULL);
>> + sqlite3_bind_int64(priv->stmt_cxl_dram_event, idx++, ev->hpa);
>> + sqlite3_bind_text(priv->stmt_cxl_dram_event, idx++, ev->region, -1,
>NULL);
>> + sqlite3_bind_text(priv->stmt_cxl_dram_event, idx++, ev->region_uuid,
>> +-1, NULL);
>>
>> rc = sqlite3_step(priv->stmt_cxl_dram_event);
>> if (rc != SQLITE_OK && rc != SQLITE_DONE) @@ -1047,7 +1058,7 @@
>> static const struct db_table_descriptor cxl_memory_module_event_tab =
>> { int ras_store_cxl_memory_module_event(struct ras_events *ras,
>> struct ras_cxl_memory_module_event *ev)
>{
>> - int rc;
>> + int rc, idx;
>> struct sqlite3_priv *priv = ras->db_priv;
>>
>> if (!priv || !priv->stmt_cxl_memory_module_event)
>> @@ -1055,16 +1066,19 @@ int ras_store_cxl_memory_module_event(struct
>ras_events *ras,
>> log(TERM, LOG_INFO, "cxl_memory_module_event store: %p\n",
>> priv->stmt_cxl_memory_module_event);
>>
>> - ras_store_cxl_common_hdr(priv->stmt_cxl_memory_module_event,
>&ev->hdr);
>> - sqlite3_bind_int(priv->stmt_cxl_memory_module_event, 13, ev-
>>event_type);
>> - sqlite3_bind_int(priv->stmt_cxl_memory_module_event, 14, ev-
>>health_status);
>> - sqlite3_bind_int(priv->stmt_cxl_memory_module_event, 15, ev-
>>media_status);
>> - sqlite3_bind_int(priv->stmt_cxl_memory_module_event, 16, ev-
>>life_used);
>> - sqlite3_bind_int(priv->stmt_cxl_memory_module_event, 17, ev-
>>dirty_shutdown_cnt);
>> - sqlite3_bind_int(priv->stmt_cxl_memory_module_event, 18, ev-
>>cor_vol_err_cnt);
>> - sqlite3_bind_int(priv->stmt_cxl_memory_module_event, 19, ev-
>>cor_per_err_cnt);
>> - sqlite3_bind_int(priv->stmt_cxl_memory_module_event, 20, ev-
>>device_temp);
>> - sqlite3_bind_int(priv->stmt_cxl_memory_module_event, 21, ev-
>>add_status);
>> + idx = ras_store_cxl_common_hdr(priv-
>>stmt_cxl_memory_module_event, &ev->hdr);
>> + if (!idx)
>as above
>> + return 0;
>> +
>> + sqlite3_bind_int(priv->stmt_cxl_memory_module_event, idx++, ev-
>>event_type);
>> + sqlite3_bind_int(priv->stmt_cxl_memory_module_event, idx++, ev-
>>health_status);
>> + sqlite3_bind_int(priv->stmt_cxl_memory_module_event, idx++, ev-
>>media_status);
>> + sqlite3_bind_int(priv->stmt_cxl_memory_module_event, idx++, ev-
>>life_used);
>> + sqlite3_bind_int(priv->stmt_cxl_memory_module_event, idx++, ev-
>>dirty_shutdown_cnt);
>> + sqlite3_bind_int(priv->stmt_cxl_memory_module_event, idx++, ev-
>>cor_vol_err_cnt);
>> + sqlite3_bind_int(priv->stmt_cxl_memory_module_event, idx++, ev-
>>cor_per_err_cnt);
>> + sqlite3_bind_int(priv->stmt_cxl_memory_module_event, idx++, ev-
>>device_temp);
>> + sqlite3_bind_int(priv->stmt_cxl_memory_module_event, idx++,
>> +ev->add_status);
>>
>> rc = sqlite3_step(priv->stmt_cxl_memory_module_event);
>> if (rc != SQLITE_OK && rc != SQLITE_DONE)
Powered by blists - more mailing lists