Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
O
OpenXG-RAN
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
0
Merge Requests
0
CI / CD
CI / CD
Pipelines
Jobs
Schedules
Analytics
Analytics
CI / CD
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Jobs
Commits
Issue Boards
Open sidebar
canghaiwuhen
OpenXG-RAN
Commits
ee04caab
Commit
ee04caab
authored
Aug 31, 2018
by
Wang Tsu-Han
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
replacing judgment from get_nprocs() and worker flags by paralle_stage and worker_stage
parent
24c1c6e1
Changes
7
Show whitespace changes
Inline
Side-by-side
Showing
7 changed files
with
111 additions
and
45 deletions
+111
-45
openair1/PHY/LTE_TRANSPORT/dlsch_coding.c
openair1/PHY/LTE_TRANSPORT/dlsch_coding.c
+26
-11
openair1/PHY/LTE_TRANSPORT/ulsch_decoding.c
openair1/PHY/LTE_TRANSPORT/ulsch_decoding.c
+2
-2
openair1/PHY/defs_eNB.h
openair1/PHY/defs_eNB.h
+6
-0
targets/RT/USER/lte-enb.c
targets/RT/USER/lte-enb.c
+13
-12
targets/RT/USER/lte-ru.c
targets/RT/USER/lte-ru.c
+15
-14
targets/RT/USER/lte-softmodem.c
targets/RT/USER/lte-softmodem.c
+43
-2
targets/RT/USER/lte-softmodem.h
targets/RT/USER/lte-softmodem.h
+6
-4
No files found.
openair1/PHY/LTE_TRANSPORT/dlsch_coding.c
View file @
ee04caab
...
...
@@ -58,7 +58,6 @@
uint64_t deadline,
uint64_t period);*/
extern
int
codingw
;
void
free_eNB_dlsch
(
LTE_eNB_DLSCH_t
*
dlsch
)
{
...
...
@@ -615,7 +614,9 @@ int dlsch_encoding_all(PHY_VARS_eNB *eNB,
}
}
if
(
C
>=
8
&&
get_nprocs
()
>=
16
&&
codingw
)
//one main three worker
if
(
get_thread_worker_stage
())
{
if
(
C
>=
8
)
//one main three worker
{
encoding_return
=
dlsch_encoding_2threads
(
eNB
,
...
...
@@ -633,7 +634,7 @@ int dlsch_encoding_all(PHY_VARS_eNB *eNB,
i_stats
,
3
);
}
else
if
(
C
>=
6
&&
get_nprocs
()
>=
8
&&
codingw
)
//one main two worker
else
if
(
C
>=
6
)
//one main two worker
{
encoding_return
=
dlsch_encoding_2threads
(
eNB
,
...
...
@@ -651,7 +652,7 @@ int dlsch_encoding_all(PHY_VARS_eNB *eNB,
i_stats
,
2
);
}
else
if
(
C
>=
4
&&
get_nprocs
()
>=
4
&&
codingw
)
//one main one worker
else
if
(
C
>=
4
)
//one main one worker
{
encoding_return
=
dlsch_encoding_2threads
(
eNB
,
...
...
@@ -670,6 +671,20 @@ int dlsch_encoding_all(PHY_VARS_eNB *eNB,
1
);
}
else
{
encoding_return
=
dlsch_encoding
(
eNB
,
a
,
num_pdcch_symbols
,
dlsch
,
frame
,
subframe
,
rm_stats
,
te_stats
,
i_stats
);
}
}
else
{
encoding_return
=
dlsch_encoding
(
eNB
,
...
...
openair1/PHY/LTE_TRANSPORT/ulsch_decoding.c
View file @
ee04caab
...
...
@@ -45,7 +45,7 @@
#include "targets/RT/USER/rt_wrapper.h"
#include "transport_proto.h"
extern
int
codingw
;
extern
uint8_t
get_thread_worker_stage
(
void
)
;
void
free_eNB_ulsch
(
LTE_eNB_ULSCH_t
*
ulsch
)
{
...
...
@@ -723,7 +723,7 @@ int ulsch_decoding_data(PHY_VARS_eNB *eNB,int UE_id,int harq_pid,int llr8_flag)
int
ulsch_decoding_data_all
(
PHY_VARS_eNB
*
eNB
,
int
UE_id
,
int
harq_pid
,
int
llr8_flag
)
{
int
ret
=
0
;
/*if(
codingw
)
/*if(
get_thread_worker_stage()
)
{
ret = ulsch_decoding_data_2thread(eNB,UE_id,harq_pid,llr8_flag);
}
...
...
openair1/PHY/defs_eNB.h
View file @
ee04caab
...
...
@@ -888,6 +888,12 @@ typedef struct {
int
prach_I0
;
}
PHY_MEASUREMENTS_eNB
;
typedef
struct
THREAD_STRUCT_s
{
uint8_t
paralle_stage
;
uint8_t
worker_stage
;
int
core_number
;
}
THREAD_STRUCT
;
/// Top-level PHY Data Structure for eNB
typedef
struct
PHY_VARS_eNB_s
{
...
...
targets/RT/USER/lte-enb.c
View file @
ee04caab
...
...
@@ -153,7 +153,8 @@ void wakeup_prach_eNB(PHY_VARS_eNB *eNB,RU_t *ru,int frame,int subframe);
#if (RRC_VERSION >= MAKE_VERSION(14, 0, 0))
void
wakeup_prach_eNB_br
(
PHY_VARS_eNB
*
eNB
,
RU_t
*
ru
,
int
frame
,
int
subframe
);
#endif
extern
int
codingw
;
extern
uint8_t
get_thread_paralle_stage
(
void
);
extern
uint8_t
get_thread_worker_stage
(
void
);
extern
uint8_t
nfapi_mode
;
extern
void
oai_subframe_ind
(
uint16_t
sfn
,
uint16_t
sf
);
...
...
@@ -225,7 +226,7 @@ static inline int rxtx(PHY_VARS_eNB *eNB,eNB_rxtx_proc_t *proc, char *thread_nam
}
VCD_SIGNAL_DUMPER_DUMP_FUNCTION_BY_NAME
(
VCD_SIGNAL_DUMPER_FUNCTIONS_ENB_DLSCH_ULSCH_SCHEDULER
,
1
);
if
(
!
eNB
->
single_thread_flag
&&
get_nprocs
()
>=
8
){
if
(
get_thread_paralle_stage
()
==
2
){
if
(
wait_on_condition
(
&
proc
[
1
].
mutex_rxtx
,
&
proc
[
1
].
cond_rxtx
,
&
proc
[
1
].
pipe_ready
,
"wakeup_tx"
)
<
0
)
{
LOG_E
(
PHY
,
"Frame %d, subframe %d: TX1 not ready
\n
"
,
proc
[
1
].
frame_rx
,
proc
[
1
].
subframe_rx
);
return
(
-
1
);
...
...
@@ -247,7 +248,7 @@ static inline int rxtx(PHY_VARS_eNB *eNB,eNB_rxtx_proc_t *proc, char *thread_nam
/* CONFLICT RESOLUTION: BEGIN */
VCD_SIGNAL_DUMPER_DUMP_FUNCTION_BY_NAME
(
VCD_SIGNAL_DUMPER_FUNCTIONS_ENB_DLSCH_ULSCH_SCHEDULER
,
0
);
if
(
oai_exit
)
return
(
-
1
);
if
(
eNB
->
single_thread_flag
||
get_nprocs
()
<=
4
){
if
(
get_thread_paralle_stage
()
==
0
){
#ifndef PHY_TX_THREAD
phy_procedures_eNB_TX
(
eNB
,
proc
,
1
);
#endif
...
...
@@ -423,8 +424,8 @@ static void* eNB_thread_rxtx( void* param ) {
}
pthread_mutex_unlock
(
&
proc
->
mutex_rxtx
);
if
(
nfapi_mode
!=
2
){
if
(
get_
nprocs
()
>=
8
)
wakeup_tx
(
eNB
,
eNB
->
proc
.
ru_proc
);
else
if
(
get_
thread_paralle_stage
()
==
2
)
wakeup_tx
(
eNB
,
eNB
->
proc
.
ru_proc
);
else
if
(
get_thread_paralle_stage
()
==
1
)
{
phy_procedures_eNB_TX
(
eNB
,
proc
,
1
);
wakeup_txfh
(
proc
,
eNB
->
proc
.
ru_proc
);
...
...
@@ -883,7 +884,7 @@ static void* process_stats_thread(void* param) {
void
init_eNB_proc
(
int
inst
)
{
int
i
=
0
;
/*int i=0;*/
int
CC_id
;
PHY_VARS_eNB
*
eNB
;
eNB_proc_t
*
proc
;
...
...
@@ -955,7 +956,7 @@ void init_eNB_proc(int inst) {
// attr_te = &proc->attr_te;
#endif
if
(
get_
nprocs
()
>
2
&&
codingw
)
if
(
get_
thread_worker_stage
()
)
{
init_te_thread
(
eNB
);
init_td_thread
(
eNB
);
...
...
@@ -964,7 +965,7 @@ void init_eNB_proc(int inst) {
LOG_I
(
PHY
,
"eNB->single_thread_flag:%d
\n
"
,
eNB
->
single_thread_flag
);
if
(
eNB
->
single_thread_flag
=
=
0
&&
nfapi_mode
!=
2
)
{
if
(
get_thread_paralle_stage
()
!
=
0
&&
nfapi_mode
!=
2
)
{
pthread_create
(
&
proc_rxtx
[
0
].
pthread_rxtx
,
attr0
,
eNB_thread_rxtx
,
proc
);
pthread_create
(
&
proc_rxtx
[
1
].
pthread_rxtx
,
attr1
,
tx_thread
,
proc
);
}
...
...
@@ -972,13 +973,13 @@ void init_eNB_proc(int inst) {
#if (RRC_VERSION >= MAKE_VERSION(14, 0, 0))
pthread_create
(
&
proc
->
pthread_prach_br
,
attr_prach_br
,
eNB_thread_prach_br
,
eNB
);
#endif
char
name
[
16
];
/*
char name[16];
if (eNB->single_thread_flag==0) {
snprintf( name, sizeof(name), "RXTX0 %d", i );
pthread_setname_np( proc_rxtx[0].pthread_rxtx, name );
snprintf( name, sizeof(name), "RXTX1 %d", i );
pthread_setname_np( proc_rxtx[1].pthread_rxtx, name );
}
}
*/
AssertFatal
(
proc
->
instance_cnt_prach
==
-
1
,
"instance_cnt_prach = %d
\n
"
,
proc
->
instance_cnt_prach
);
...
...
@@ -1311,8 +1312,8 @@ void init_eNB(int single_thread_flag,int wait_for_sync) {
#endif
eNB
->
td
=
ulsch_decoding_data_all
;
//(get_nprocs()<=4) ? ulsch_decoding_data : ulsch_decoding_data_2thread;
eNB
->
te
=
dlsch_encoding_all
;
//(get_nprocs()<=4) ? dlsch_encoding : dlsch_encoding_2threads;
eNB
->
td
=
ulsch_decoding_data_all
;
eNB
->
te
=
dlsch_encoding_all
;
LOG_I
(
PHY
,
"Registering with MAC interface module
\n
"
);
...
...
targets/RT/USER/lte-ru.c
View file @
ee04caab
...
...
@@ -119,10 +119,11 @@ static int DEFENBS[] = {0};
extern
volatile
int
oai_exit
;
extern
int
emulate_rf
;
extern
int
numerology
;
extern
int
fepw
;
extern
int
single_thread_flag
;
extern
clock_source_t
clock_source
;
extern
uint8_t
get_thread_paralle_stage
(
void
);
extern
uint8_t
get_thread_worker_stage
(
void
);
extern
int
get_thread_core_number
(
void
);
extern
void
phy_init_RU
(
RU_t
*
);
extern
void
phy_free_RU
(
RU_t
*
);
...
...
@@ -1257,7 +1258,7 @@ void wakeup_eNBs(RU_t *ru) {
LOG_D
(
PHY
,
"wakeup_eNBs (num %d) for RU %d ru->eNB_top:%p
\n
"
,
ru
->
num_eNB
,
ru
->
idx
,
ru
->
eNB_top
);
if
(
ru
->
num_eNB
==
1
&&
ru
->
eNB_top
!=
0
&&
(
get_nprocs
()
<=
4
||
single_thread_flag
)
)
{
if
(
ru
->
num_eNB
==
1
&&
ru
->
eNB_top
!=
0
&&
get_thread_paralle_stage
()
==
0
)
{
// call eNB function directly
char
string
[
20
];
...
...
@@ -1782,7 +1783,7 @@ static void* ru_thread( void* param ) {
if
(
ru
->
num_eNB
>
0
)
wakeup_eNBs
(
ru
);
#ifndef PHY_TX_THREAD
if
(
get_
nprocs
()
<=
4
||
ru
->
num_eNB
==
0
||
single_thread_flag
){
if
(
get_
thread_paralle_stage
()
==
0
||
ru
->
num_eNB
==
0
){
// do TX front-end processing if needed (precoding and/or IDFTs)
if
(
ru
->
feptx_prec
)
ru
->
feptx_prec
(
ru
);
...
...
@@ -2195,7 +2196,7 @@ void init_RU_proc(RU_t *ru) {
if
(
emulate_rf
)
pthread_create
(
&
proc
->
pthread_emulateRF
,
attr_emulateRF
,
emulatedRF_thread
,
(
void
*
)
proc
);
if
(
!
single_thread_flag
&&
get_nprocs
()
>
4
)
if
(
get_thread_paralle_stage
()
!=
0
)
pthread_create
(
&
proc
->
pthread_FH1
,
attr_FH1
,
ru_thread_tx
,
(
void
*
)
ru
);
if
(
ru
->
function
==
NGFI_RRU_IF4p5
)
{
...
...
@@ -2222,7 +2223,7 @@ void init_RU_proc(RU_t *ru) {
pthread_create
(
&
proc
->
pthread_prach
,
attr_prach
,
ru_thread_prach
,
(
void
*
)
ru
);
}
if
(
get_
nprocs
()
>
2
&&
fepw
)
{
if
(
get_
thread_worker_stage
()
)
{
init_fep_thread
(
ru
,
NULL
);
init_feptx_thread
(
ru
,
NULL
);
}
...
...
@@ -2235,7 +2236,7 @@ void kill_RU_proc(int inst)
RU_t
*
ru
=
RC
.
ru
[
inst
];
RU_proc_t
*
proc
=
&
ru
->
proc
;
if
(
get_
nprocs
()
>
2
&&
fepw
)
{
if
(
get_
thread_worker_stage
()
)
{
LOG_D
(
PHY
,
"killing FEP thread
\n
"
);
kill_fep_thread
(
ru
);
LOG_D
(
PHY
,
"killing FEP TX thread
\n
"
);
...
...
@@ -2554,8 +2555,8 @@ void set_function_spec_param(RU_t *ru)
ru
->
fh_north_out
=
fh_if4p5_north_out
;
// send_IF4p5 on reception
ru
->
fh_south_out
=
tx_rf
;
// send output to RF
ru
->
fh_north_asynch_in
=
fh_if4p5_north_asynch_in
;
// TX packets come asynchronously
ru
->
feprx
=
(
get_nprocs
()
<=
2
||
!
fepw
)
?
fep_full
:
ru_fep_full_2thread
;
// RX DFTs
ru
->
feptx_ofdm
=
(
get_nprocs
()
<=
2
||
!
fepw
)
?
feptx_ofdm
:
feptx_ofdm_2thread
;
// this is fep with idft only (no precoding in RRU)
ru
->
feprx
=
(
!
get_thread_worker_stage
()
)
?
fep_full
:
ru_fep_full_2thread
;
// RX DFTs
ru
->
feptx_ofdm
=
(
!
get_thread_worker_stage
()
)
?
feptx_ofdm
:
feptx_ofdm_2thread
;
// this is fep with idft only (no precoding in RRU)
ru
->
feptx_prec
=
NULL
;
ru
->
start_if
=
start_if
;
// need to start the if interface for if4p5
ru
->
ifdevice
.
host_type
=
RRU_HOST
;
...
...
@@ -2576,8 +2577,8 @@ void set_function_spec_param(RU_t *ru)
}
else
if
(
ru
->
function
==
eNodeB_3GPP
)
{
ru
->
do_prach
=
0
;
// no prach processing in RU
ru
->
feprx
=
(
get_nprocs
()
<=
2
||
!
fepw
)
?
fep_full
:
ru_fep_full_2thread
;
// RX DFTs
ru
->
feptx_ofdm
=
(
get_nprocs
()
<=
2
||
!
fepw
)
?
feptx_ofdm
:
feptx_ofdm_2thread
;
// this is fep with idft and precoding
ru
->
feprx
=
(
!
get_thread_worker_stage
()
)
?
fep_full
:
ru_fep_full_2thread
;
// RX DFTs
ru
->
feptx_ofdm
=
(
!
get_thread_worker_stage
()
)
?
feptx_ofdm
:
feptx_ofdm_2thread
;
// this is fep with idft and precoding
ru
->
feptx_prec
=
feptx_prec
;
// this is fep with idft and precoding
ru
->
fh_north_in
=
NULL
;
// no incoming fronthaul from north
ru
->
fh_north_out
=
NULL
;
// no outgoing fronthaul to north
...
...
@@ -2605,9 +2606,9 @@ void set_function_spec_param(RU_t *ru)
case
REMOTE_IF5
:
// the remote unit is IF5 RRU
ru
->
do_prach
=
0
;
ru
->
feprx
=
(
get_nprocs
()
<=
2
||
!
fepw
)
?
fep_full
:
fep_full
;
// this is frequency-shift + DFTs
ru
->
feprx
=
(
!
get_thread_worker_stage
()
)
?
fep_full
:
fep_full
;
// this is frequency-shift + DFTs
ru
->
feptx_prec
=
feptx_prec
;
// need to do transmit Precoding + IDFTs
ru
->
feptx_ofdm
=
(
get_nprocs
()
<=
2
||
!
fepw
)
?
feptx_ofdm
:
feptx_ofdm_2thread
;
// need to do transmit Precoding + IDFTs
ru
->
feptx_ofdm
=
(
!
get_thread_worker_stage
()
)
?
feptx_ofdm
:
feptx_ofdm_2thread
;
// need to do transmit Precoding + IDFTs
if
(
ru
->
if_timing
==
synch_to_other
)
{
ru
->
fh_south_in
=
fh_slave_south_in
;
// synchronize to master
ru
->
fh_south_out
=
fh_if5_mobipass_south_out
;
// use send_IF5 for mobipass
...
...
@@ -2685,7 +2686,7 @@ void init_RU(char *rf_config_file) {
// read in configuration file)
printf
(
"configuring RU from file
\n
"
);
RCconfig_RU
();
LOG_I
(
PHY
,
"number of L1 instances %d, number of RU %d, number of CPU cores %d
\n
"
,
RC
.
nb_L1_inst
,
RC
.
nb_RU
,
get_
nprocs
());
LOG_I
(
PHY
,
"number of L1 instances %d, number of RU %d, number of CPU cores %d
\n
"
,
RC
.
nb_L1_inst
,
RC
.
nb_RU
,
get_
thread_core_number
());
if
(
RC
.
nb_CC
!=
0
)
for
(
i
=
0
;
i
<
RC
.
nb_L1_inst
;
i
++
)
...
...
targets/RT/USER/lte-softmodem.c
View file @
ee04caab
...
...
@@ -216,8 +216,48 @@ extern void init_eNB_afterRU(void);
int
transmission_mode
=
1
;
int
emulate_rf
=
0
;
int
numerology
=
0
;
int
codingw
=
0
;
int
fepw
=
0
;
int
dis_worker
=
1
;
static
THREAD_STRUCT
thread_struct
;
void
thread_structure_init
(
void
)
{
thread_struct
.
core_number
=
get_nprocs
();
if
(
thread_struct
.
core_number
>=
8
)
{
thread_struct
.
paralle_stage
=
2
;
thread_struct
.
worker_stage
=
1
;
}
else
if
(
thread_struct
.
core_number
>=
4
)
{
thread_struct
.
paralle_stage
=
1
;
thread_struct
.
worker_stage
=
1
;
}
else
{
thread_struct
.
paralle_stage
=
0
;
thread_struct
.
worker_stage
=
0
;
}
if
(
single_thread_flag
)
thread_struct
.
paralle_stage
=
0
;
if
(
dis_worker
)
thread_struct
.
worker_stage
=
0
;
printf
(
"single_thread_flag=%d, dis_worker=%d
\n
"
,
single_thread_flag
,
dis_worker
);
printf
(
"~~~~~~~~~~paralle_stage=%d, worker_stage=%d, core_number=%d ~~~~~~~~~~~~~~~~
\n
"
,
thread_struct
.
paralle_stage
,
thread_struct
.
worker_stage
,
thread_struct
.
core_number
);
}
uint8_t
get_thread_paralle_stage
(
void
)
{
return
thread_struct
.
paralle_stage
;
}
uint8_t
get_thread_worker_stage
(
void
)
{
return
thread_struct
.
worker_stage
;
}
int
get_thread_core_number
(
void
)
{
return
thread_struct
.
core_number
;
}
...
...
@@ -1174,6 +1214,7 @@ int main( int argc, char **argv )
printf
(
"wait_eNBs()
\n
"
);
wait_eNBs
();
thread_structure_init
();
printf
(
"About to Init RU threads RC.nb_RU:%d
\n
"
,
RC
.
nb_RU
);
if
(
RC
.
nb_RU
>
0
)
{
...
...
targets/RT/USER/lte-softmodem.h
View file @
ee04caab
...
...
@@ -92,8 +92,7 @@
#define CONFIG_HLP_TNOFORK "to ease debugging with gdb\n"
#define CONFIG_HLP_NUMEROLOGY "adding numerology for 5G\n"
#define CONFIG_HLP_CODINGW "coding worker thread enable(disable by defult)\n"
#define CONFIG_HLP_FEPW "FEP worker thread enabled(disable by defult)\n"
#define CONFIG_HLP_WORKER "coding and FEP worker thread disable(enable by defult)\n"
#define CONFIG_HLP_EMULATE_RF "Emulated RF enabled(disable by defult)\n"
#define CONFIG_HLP_DISABLNBIOT "disable nb-iot, even if defined in config\n"
...
...
@@ -186,8 +185,7 @@
{"s" , CONFIG_HLP_SNR, 0, iptr:&snr_dB, defintval:25, TYPE_INT, 0}, \
{"numerology" , CONFIG_HLP_NUMEROLOGY, PARAMFLAG_BOOL, iptr:&numerology, defintval:0, TYPE_INT, 0}, \
{"emulate-rf" , CONFIG_HLP_EMULATE_RF, PARAMFLAG_BOOL, iptr:&emulate_rf, defintval:0, TYPE_INT, 0}, \
{"codingw" , CONFIG_HLP_CODINGW, PARAMFLAG_BOOL, iptr:&codingw, defintval:0, TYPE_INT, 0}, \
{"fepw" , CONFIG_HLP_FEPW, PARAMFLAG_BOOL, iptr:&fepw, defintval:0, TYPE_INT, 0}, \
{"worker-disable" , CONFIG_HLP_WORKER, PARAMFLAG_BOOL, iptr:&dis_worker, defintval:0, TYPE_INT, 0}, \
{"nbiot-disable", CONFIG_HLP_DISABLNBIOT, PARAMFLAG_BOOL, iptr:&nonbiotflag, defintval:0, TYPE_INT, 0} \
}
...
...
@@ -278,6 +276,10 @@ PHY_VARS_UE* init_ue_vars(LTE_DL_FRAME_PARMS *frame_parms,
uint8_t
UE_id
,
uint8_t
abstraction_flag
);
void
init_eNB_afterRU
(
void
);
void
thread_structure_init
(
void
);
uint8_t
get_thread_paralle_stage
(
void
);
uint8_t
get_thread_worker_stage
(
void
);
int
get_thread_core_number
(
void
);
extern
int
stop_L1L2
(
module_id_t
enb_id
);
extern
int
restart_L1L2
(
module_id_t
enb_id
);
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment